From 8c4e389a1fdbd061656bcdd8cd8c3ff8620249ca Mon Sep 17 00:00:00 2001 From: younggglcy Date: Tue, 9 Sep 2025 00:10:14 +0800 Subject: [PATCH 1/8] feat: silent success by default --- cli.js | 8 +++++++- simple-git-hooks.js | 29 ++++++++++++++++++++++++++--- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/cli.js b/cli.js index 6bb068a..dcbb385 100755 --- a/cli.js +++ b/cli.js @@ -11,5 +11,11 @@ if(skipInstall()) { } setHooksFromConfig(process.cwd(), process.argv) - .then(() => console.log('[INFO] Successfully set all git hooks')) + .then(({ hasContentsChanged }) => { + if (!hasContentsChanged) { + // all configured hooks are set but with same contents, silent success + return + } + console.log('[INFO] Successfully set all git hooks') + }) .catch(e => console.log('[ERROR], Was not able to set git hooks. Error: ' + e)) diff --git a/simple-git-hooks.js b/simple-git-hooks.js index 67595eb..a37bae7 100644 --- a/simple-git-hooks.js +++ b/simple-git-hooks.js @@ -176,13 +176,25 @@ async function setHooksFromConfig(projectRootPath=process.cwd(), argv=process.ar const preserveUnused = Array.isArray(config.preserveUnused) ? config.preserveUnused : config.preserveUnused ? VALID_GIT_HOOKS: [] + let hasContentsChanged = false + for (let hook of VALID_GIT_HOOKS) { if (Object.prototype.hasOwnProperty.call(config, hook)) { - _setHook(hook, config[hook], projectRootPath) + const isHookSet = _setHook(hook, config[hook], projectRootPath) + if (isHookSet) { + hasContentsChanged = true + } } else if (!preserveUnused.includes(hook)) { - _removeHook(hook, projectRootPath) + const isHookRemoved = _removeHook(hook, projectRootPath) + if (isHookRemoved) { + hasContentsChanged = true + } } } + + return { + hasContentsChanged, + } } /** @@ -219,6 +231,7 @@ function _getHooksDirPath(projectRoot) { * @param {string} hook * @param {string} command * @param {string} projectRoot + * @returns {boolean} whether the hook was set successfully, false if it was already set with same contents * @private */ function _setHook(hook, command, projectRoot=process.cwd()) { @@ -226,7 +239,7 @@ function _setHook(hook, command, projectRoot=process.cwd()) { if (!gitRoot) { console.info('[INFO] No `.git` root folder found, skipping') - return + return false } const hookCommand = PREPEND_SCRIPT + command @@ -238,10 +251,17 @@ function _setHook(hook, command, projectRoot=process.cwd()) { fs.mkdirSync(normalizedHookDirectory, { recursive: true }) } + if (fs.existsSync(hookPath)) { + const existingHook = fs.readFileSync(hookPath, { encoding: 'utf-8' }) + if (existingHook === hookCommand) { + return false + } + } fs.writeFileSync(hookPath, hookCommand) fs.chmodSync(hookPath, 0o0755) console.info(`[INFO] Successfully set the ${hook} with command: ${command}`) + return true } /** @@ -268,6 +288,7 @@ async function removeHooks(projectRoot = process.cwd()) { * Removes the pre-commit hook * @param {string} hook * @param {string} projectRoot + * @returns {boolean} whether the hook was removed * @private */ function _removeHook(hook, projectRoot=process.cwd()) { @@ -276,7 +297,9 @@ function _removeHook(hook, projectRoot=process.cwd()) { if (fs.existsSync(hookPath)) { fs.unlinkSync(hookPath) + return true } + return false } /** Reads package.json file, returns package.json content and path From 6a024c2b3027b300171f11730bc9374724ee338d Mon Sep 17 00:00:00 2001 From: younggglcy Date: Tue, 9 Sep 2025 00:37:06 +0800 Subject: [PATCH 2/8] test: add e2e test case --- simple-git-hooks.test.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/simple-git-hooks.test.js b/simple-git-hooks.test.js index f126c8b..8e92fdc 100644 --- a/simple-git-hooks.test.js +++ b/simple-git-hooks.test.js @@ -557,6 +557,15 @@ describe("Simple Git Hooks tests", () => { expect(JSON.stringify(installedHooks)).toBe(JSON.stringify(COMMON_GIT_HOOKS)); expect(isEqual(installedHooks, COMMON_GIT_HOOKS)).toBe(true); }); + + it(`should silently succeed if all hooks are already set for command: ${args.join(" ")}`, async () => { + createGitHooksFolder(PROJECT_WITH_CUSTOM_CONF); + + await simpleGitHooks.setHooksFromConfig(PROJECT_WITH_CUSTOM_CONF, args); + const consoleSpy = jest.spyOn(console, 'log').mockImplementation(() => {}); + await simpleGitHooks.setHooksFromConfig(PROJECT_WITH_CUSTOM_CONF, args); + expect(consoleSpy).not.toHaveBeenCalled(); + }); }); describe("SKIP_INSTALL_SIMPLE_GIT_HOOKS", () => { From 09cc3e1a3e0a011dc5354faaf3f924532b117718 Mon Sep 17 00:00:00 2001 From: younggglcy Date: Tue, 9 Sep 2025 01:09:32 +0800 Subject: [PATCH 3/8] chore: changeset --- .changeset/easy-sites-stick.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/easy-sites-stick.md diff --git a/.changeset/easy-sites-stick.md b/.changeset/easy-sites-stick.md new file mode 100644 index 0000000..b14a538 --- /dev/null +++ b/.changeset/easy-sites-stick.md @@ -0,0 +1,5 @@ +--- +"simple-git-hooks": minor +--- + +feat: silent success if nothing has changed From cd974e51fc8fc8971f142e793b3ed651518a9d86 Mon Sep 17 00:00:00 2001 From: younggglcy Date: Tue, 9 Sep 2025 02:05:40 +0800 Subject: [PATCH 4/8] fix: correct logics and fix tests --- simple-git-hooks.js | 8 ++------ simple-git-hooks.test.js | 22 +++++++++++++--------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/simple-git-hooks.js b/simple-git-hooks.js index a37bae7..eb84325 100644 --- a/simple-git-hooks.js +++ b/simple-git-hooks.js @@ -181,14 +181,10 @@ async function setHooksFromConfig(projectRootPath=process.cwd(), argv=process.ar for (let hook of VALID_GIT_HOOKS) { if (Object.prototype.hasOwnProperty.call(config, hook)) { const isHookSet = _setHook(hook, config[hook], projectRootPath) - if (isHookSet) { - hasContentsChanged = true - } + hasContentsChanged = hasContentsChanged || isHookSet } else if (!preserveUnused.includes(hook)) { const isHookRemoved = _removeHook(hook, projectRootPath) - if (isHookRemoved) { - hasContentsChanged = true - } + hasContentsChanged = hasContentsChanged || isHookRemoved } } diff --git a/simple-git-hooks.test.js b/simple-git-hooks.test.js index 8e92fdc..ee3a3d8 100644 --- a/simple-git-hooks.test.js +++ b/simple-git-hooks.test.js @@ -557,15 +557,6 @@ describe("Simple Git Hooks tests", () => { expect(JSON.stringify(installedHooks)).toBe(JSON.stringify(COMMON_GIT_HOOKS)); expect(isEqual(installedHooks, COMMON_GIT_HOOKS)).toBe(true); }); - - it(`should silently succeed if all hooks are already set for command: ${args.join(" ")}`, async () => { - createGitHooksFolder(PROJECT_WITH_CUSTOM_CONF); - - await simpleGitHooks.setHooksFromConfig(PROJECT_WITH_CUSTOM_CONF, args); - const consoleSpy = jest.spyOn(console, 'log').mockImplementation(() => {}); - await simpleGitHooks.setHooksFromConfig(PROJECT_WITH_CUSTOM_CONF, args); - expect(consoleSpy).not.toHaveBeenCalled(); - }); }); describe("SKIP_INSTALL_SIMPLE_GIT_HOOKS", () => { @@ -654,6 +645,19 @@ describe("Simple Git Hooks tests", () => { expect(installedHooks).toEqual({ "pre-commit": TEST_SCRIPT }); }); }); + + describe('should silently succeed if all hooks are already', () => { + it('silent success by default', async () => { + createGitHooksFolder(PROJECT_WITH_CONF_IN_SEPARATE_JSON); + + const output1 = execSync(`node ${require.resolve("./cli")}`, { cwd: PROJECT_WITH_CONF_IN_SEPARATE_JSON, encoding: 'utf8' }); + expect(output1).toMatch(/\[INFO\] Successfully set all git hooks/); + expect(output1).toMatch(/\[INFO\] Successfully set the pre-commit with command: exit 1/); + + const output2 = execSync(`node ${require.resolve("./cli")}`, { cwd: PROJECT_WITH_CONF_IN_SEPARATE_JSON, encoding: 'utf8' }).trim(); + expect(output2).toBe(''); + }) + }); }); describe("ENV vars features tests", () => { From 5bad651d5cd0ef7cc1761cbb7cde57f5a3fb76f2 Mon Sep 17 00:00:00 2001 From: younggglcy Date: Sat, 13 Sep 2025 01:59:31 +0800 Subject: [PATCH 5/8] fix: apply reviews --- cli.js | 4 ++-- simple-git-hooks.js | 26 +++++++++++++++++--------- simple-git-hooks.test.js | 2 +- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/cli.js b/cli.js index dcbb385..bb64575 100755 --- a/cli.js +++ b/cli.js @@ -11,8 +11,8 @@ if(skipInstall()) { } setHooksFromConfig(process.cwd(), process.argv) - .then(({ hasContentsChanged }) => { - if (!hasContentsChanged) { + .then(({ isHookChanged }) => { + if (!isHookChanged) { // all configured hooks are set but with same contents, silent success return } diff --git a/simple-git-hooks.js b/simple-git-hooks.js index eb84325..6c697ac 100644 --- a/simple-git-hooks.js +++ b/simple-git-hooks.js @@ -176,20 +176,20 @@ async function setHooksFromConfig(projectRootPath=process.cwd(), argv=process.ar const preserveUnused = Array.isArray(config.preserveUnused) ? config.preserveUnused : config.preserveUnused ? VALID_GIT_HOOKS: [] - let hasContentsChanged = false + let isHookChanged = false for (let hook of VALID_GIT_HOOKS) { if (Object.prototype.hasOwnProperty.call(config, hook)) { - const isHookSet = _setHook(hook, config[hook], projectRootPath) - hasContentsChanged = hasContentsChanged || isHookSet + const isHookUpdated = _setHook(hook, config[hook], projectRootPath).hookChanged + isHookChanged = isHookChanged || isHookUpdated } else if (!preserveUnused.includes(hook)) { const isHookRemoved = _removeHook(hook, projectRootPath) - hasContentsChanged = hasContentsChanged || isHookRemoved + isHookChanged = isHookChanged || isHookRemoved } } return { - hasContentsChanged, + isHookChanged, } } @@ -227,7 +227,6 @@ function _getHooksDirPath(projectRoot) { * @param {string} hook * @param {string} command * @param {string} projectRoot - * @returns {boolean} whether the hook was set successfully, false if it was already set with same contents * @private */ function _setHook(hook, command, projectRoot=process.cwd()) { @@ -235,7 +234,10 @@ function _setHook(hook, command, projectRoot=process.cwd()) { if (!gitRoot) { console.info('[INFO] No `.git` root folder found, skipping') - return false + return { + hookChanged: false, + success: false + } } const hookCommand = PREPEND_SCRIPT + command @@ -250,14 +252,20 @@ function _setHook(hook, command, projectRoot=process.cwd()) { if (fs.existsSync(hookPath)) { const existingHook = fs.readFileSync(hookPath, { encoding: 'utf-8' }) if (existingHook === hookCommand) { - return false + return { + hookChanged: false, + success: true + } } } fs.writeFileSync(hookPath, hookCommand) fs.chmodSync(hookPath, 0o0755) console.info(`[INFO] Successfully set the ${hook} with command: ${command}`) - return true + return { + hookChanged: true, + success: true + } } /** diff --git a/simple-git-hooks.test.js b/simple-git-hooks.test.js index ee3a3d8..b9b47a1 100644 --- a/simple-git-hooks.test.js +++ b/simple-git-hooks.test.js @@ -646,7 +646,7 @@ describe("Simple Git Hooks tests", () => { }); }); - describe('should silently succeed if all hooks are already', () => { + describe('should silently succeed if all hooks are already installed', () => { it('silent success by default', async () => { createGitHooksFolder(PROJECT_WITH_CONF_IN_SEPARATE_JSON); From 1c335376eba8288518178d6c1fc54051aa867a62 Mon Sep 17 00:00:00 2001 From: younggglcy Date: Sat, 13 Sep 2025 03:20:57 +0800 Subject: [PATCH 6/8] feat: improve readability --- simple-git-hooks.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/simple-git-hooks.js b/simple-git-hooks.js index 6c697ac..6ec0bed 100644 --- a/simple-git-hooks.js +++ b/simple-git-hooks.js @@ -176,18 +176,19 @@ async function setHooksFromConfig(projectRootPath=process.cwd(), argv=process.ar const preserveUnused = Array.isArray(config.preserveUnused) ? config.preserveUnused : config.preserveUnused ? VALID_GIT_HOOKS: [] - let isHookChanged = false + let isHookUpdated = false + let isHookDeleted = false for (let hook of VALID_GIT_HOOKS) { if (Object.prototype.hasOwnProperty.call(config, hook)) { - const isHookUpdated = _setHook(hook, config[hook], projectRootPath).hookChanged - isHookChanged = isHookChanged || isHookUpdated + isHookUpdated = _setHook(hook, config[hook], projectRootPath).hookChanged } else if (!preserveUnused.includes(hook)) { - const isHookRemoved = _removeHook(hook, projectRootPath) - isHookChanged = isHookChanged || isHookRemoved + isHookDeleted = _removeHook(hook, projectRootPath) } } + const isHookChanged = isHookUpdated || isHookDeleted + return { isHookChanged, } From 28ff96c6008cb69250f6c66efba3861cdb69965c Mon Sep 17 00:00:00 2001 From: younggglcy Date: Sat, 13 Sep 2025 16:10:08 +0800 Subject: [PATCH 7/8] fix: correct logic --- simple-git-hooks.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/simple-git-hooks.js b/simple-git-hooks.js index 6ec0bed..63414e3 100644 --- a/simple-git-hooks.js +++ b/simple-git-hooks.js @@ -181,9 +181,9 @@ async function setHooksFromConfig(projectRootPath=process.cwd(), argv=process.ar for (let hook of VALID_GIT_HOOKS) { if (Object.prototype.hasOwnProperty.call(config, hook)) { - isHookUpdated = _setHook(hook, config[hook], projectRootPath).hookChanged + isHookUpdated = isHookUpdated || _setHook(hook, config[hook], projectRootPath).hookChanged } else if (!preserveUnused.includes(hook)) { - isHookDeleted = _removeHook(hook, projectRootPath) + isHookDeleted = isHookDeleted || _removeHook(hook, projectRootPath) } } From e49d1af5b6ced123db87c7a3d2f13de31f51aac7 Mon Sep 17 00:00:00 2001 From: younggglcy Date: Sat, 13 Sep 2025 23:10:49 +0800 Subject: [PATCH 8/8] fix: update hook change detection logic --- simple-git-hooks.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/simple-git-hooks.js b/simple-git-hooks.js index 63414e3..7ad6d60 100644 --- a/simple-git-hooks.js +++ b/simple-git-hooks.js @@ -176,18 +176,18 @@ async function setHooksFromConfig(projectRootPath=process.cwd(), argv=process.ar const preserveUnused = Array.isArray(config.preserveUnused) ? config.preserveUnused : config.preserveUnused ? VALID_GIT_HOOKS: [] - let isHookUpdated = false - let isHookDeleted = false + let isHookChanged = false for (let hook of VALID_GIT_HOOKS) { if (Object.prototype.hasOwnProperty.call(config, hook)) { - isHookUpdated = isHookUpdated || _setHook(hook, config[hook], projectRootPath).hookChanged + const isHookUpdated = _setHook(hook, config[hook], projectRootPath).hookChanged + isHookChanged = isHookChanged || isHookUpdated } else if (!preserveUnused.includes(hook)) { - isHookDeleted = isHookDeleted || _removeHook(hook, projectRootPath) + const isHookDeleted = _removeHook(hook, projectRootPath) + isHookChanged = isHookChanged || isHookDeleted } } - const isHookChanged = isHookUpdated || isHookDeleted return { isHookChanged,