ReplaceBranch adds trailing newline to commit message

[cmackenzie1]

ReplaceBranch adds a trailing newline to the commit message if using -m flag.

Steps to reproduce

1. Enable ReplaceBranch

PrepareCommitMsg:
  ReplaceBranch:
    enabled: true
    description: 'Prepends the commit message with text based on the branch name'
    branch_pattern: '(.*?)(?<ticket>[A-Z][A-Z0-9]+-[0-9]+)(.*)'
    replacement_text: '\k<ticket> | '
    skipped_commit_types: ['template', 'merge', 'squash', 'rebase']
    on_fail: warn

2. Install overcommit hook
3. Create a branch with a ticket id in it. ie, FOO-123
4. git add and git commit -m "some message"

Result

> OVERCOMMIT_DEBUG=1 git commit -m "Should not have preceding newline"
git rev-parse --show-toplevel
EXIT STATUS: 0
STDOUT: "/Users/cmackenzie1/work/json-faker-lambda\n"
STDERR: ""
git rev-parse --git-dir
EXIT STATUS: 0
STDOUT: ".git\n"
STDERR: ""
git rev-parse --git-common-dir
EXIT STATUS: 0
STDOUT: ".git\n"
STDERR: ""
git config --local --get overcommit.configuration.verifysignatures
EXIT STATUS: 0
STDOUT: "0\n"
STDERR: ""
git config --local --get overcommit.configuration.verifysignatures
EXIT STATUS: 0
STDOUT: "0\n"
STDERR: ""
git rev-parse HEAD
EXIT STATUS: 0
STDOUT: "a53cf70ea5e8b73311e48574d7a3f9dba9872884\n"
STDERR: ""
git --no-pager diff --quiet
EXIT STATUS: 0
STDOUT: ""
STDERR: ""
Running pre-commit hooks
git rev-parse HEAD
grep -IHn ^<<<<<<<[     ] ... (1 splittable args)
EXIT STATUS: 1
STDOUT: ""
STDERR: ""
EXIT STATUS: 0
STDOUT: "a53cf70ea5e8b73311e48574d7a3f9dba9872884\n"
STDERR: ""

✓ All pre-commit hooks passed

git rev-parse --show-toplevel
EXIT STATUS: 0
STDOUT: "/Users/cmackenzie1/work/json-faker-lambda\n"
STDERR: ""
git rev-parse --git-dir
EXIT STATUS: 0
STDOUT: ".git\n"
STDERR: ""
git rev-parse --git-common-dir
EXIT STATUS: 0
STDOUT: ".git\n"
STDERR: ""
git config --local --get overcommit.configuration.verifysignatures
EXIT STATUS: 0
STDOUT: "0\n"
STDERR: ""
git config --local --get overcommit.configuration.verifysignatures
EXIT STATUS: 0
STDOUT: "0\n"
STDERR: ""
Running prepare-commit-msg hooks
Checking if 'PROX-3049-test-event-generation' matches (?-mix:(.*?)(?<ticket>[A-Z][A-Z0-9]+-[0-9]+)(.*))
Writing .git/COMMIT_EDITMSG with PROX-3049 | 
Prepends the commit message with text based on the branch name[ReplaceBranch] OK

✓ All prepare-commit-msg hooks passed

git rev-parse --show-toplevel
EXIT STATUS: 0
STDOUT: "/Users/cmackenzie1/work/json-faker-lambda\n"
STDERR: ""
git rev-parse --git-dir
EXIT STATUS: 0
STDOUT: ".git\n"
STDERR: ""
git rev-parse --git-common-dir
EXIT STATUS: 0
STDOUT: ".git\n"
STDERR: ""
git config --local --get overcommit.configuration.verifysignatures
EXIT STATUS: 0
STDOUT: "0\n"
STDERR: ""
git config --local --get overcommit.configuration.verifysignatures
EXIT STATUS: 0
STDOUT: "0\n"
STDERR: ""
Running commit-msg hooks
Check text width..........................................[TextWidth] OK
Check commit message matches expected pattern.........[MessageFormat] OK
Check subject capitalization.....................[CapitalizedSubject] OK
Check subject line................................[SingleLineSubject] WARNING
Subject should be one line and followed by a blank line
Check for trailing periods in subject................[TrailingPeriod] OK

⚠ All commit-msg hooks passed, but with warnings

git rev-parse --show-toplevel
EXIT STATUS: 0
STDOUT: "/Users/cmackenzie1/work/json-faker-lambda\n"
STDERR: ""
git rev-parse --git-dir
EXIT STATUS: 0
STDOUT: ".git\n"
STDERR: ""
git rev-parse --git-common-dir
EXIT STATUS: 0
STDOUT: ".git\n"
STDERR: ""
git config --local --get overcommit.configuration.verifysignatures
EXIT STATUS: 0
STDOUT: "0\n"
STDERR: ""
git config --local --get overcommit.configuration.verifysignatures
EXIT STATUS: 0
STDOUT: "0\n"
STDERR: ""
✓ No applicable post-commit hooks to run
[PROX-3049-test-event-generation aef968d] PROX-3049 | Should not have preceding newline
 1 file changed, 1 deletion(-)

> git log
commit aef968d69d8e9910af8948e10064b9797927fa56 (HEAD -> PROX-3049-test-event-generation)
Author: Cole Mackenzie <cole_mackenzie@intuit.com>
Date:   Wed Feb 19 15:52:09 2020 -0800

    PROX-3049 |
    Should not have preceding newline

Suggested change

Remove the newline separator between old and new contents and make the user specify if they want a newline in replacement_text. Trying it myself it yielded the desired result.

https://github.com/sds/overcommit/blob/master/lib/overcommit/hook/prepare_commit_msg/replace_branch.rb#L20

> OVERCOMMIT_DEBUG=1 git commit -m "Does not have trailing newline"  
git rev-parse --show-toplevel
EXIT STATUS: 0
STDOUT: "/Users/cmackenzie1/work/json-faker-lambda\n"
STDERR: ""
git rev-parse --git-dir
EXIT STATUS: 0
STDOUT: ".git\n"
STDERR: ""
git rev-parse --git-common-dir
EXIT STATUS: 0
STDOUT: ".git\n"
STDERR: ""
git config --local --get overcommit.configuration.verifysignatures
EXIT STATUS: 0
STDOUT: "0\n"
STDERR: ""
git config --local --get overcommit.configuration.verifysignatures
EXIT STATUS: 0
STDOUT: "0\n"
STDERR: ""
git rev-parse HEAD
EXIT STATUS: 0
STDOUT: "1ce27e2872e8a066ffec42511eaeceb402f6f256\n"
STDERR: ""
git --no-pager diff --quiet
EXIT STATUS: 0
STDOUT: ""
STDERR: ""
Running pre-commit hooks
git rev-parse HEAD
grep -IHn ^<<<<<<<[     ] ... (1 splittable args)
EXIT STATUS: 1
STDOUT: ""
STDERR: ""
EXIT STATUS: 0
STDOUT: "1ce27e2872e8a066ffec42511eaeceb402f6f256\n"
STDERR: ""

✓ All pre-commit hooks passed

git rev-parse --show-toplevel
EXIT STATUS: 0
STDOUT: "/Users/cmackenzie1/work/json-faker-lambda\n"
STDERR: ""
git rev-parse --git-dir
EXIT STATUS: 0
STDOUT: ".git\n"
STDERR: ""
git rev-parse --git-common-dir
EXIT STATUS: 0
STDOUT: ".git\n"
STDERR: ""
git config --local --get overcommit.configuration.verifysignatures
EXIT STATUS: 0
STDOUT: "0\n"
STDERR: ""
git config --local --get overcommit.configuration.verifysignatures
EXIT STATUS: 0
STDOUT: "0\n"
STDERR: ""
Running prepare-commit-msg hooks
Checking if 'PROX-3049-test-event-generation' matches (?-mix:(.*?)(?<ticket>[A-Z][A-Z0-9]+-[0-9]+)(.*))
Writing .git/COMMIT_EDITMSG with PROX-3049 | 
Prepends the commit message with text based on the branch name[ReplaceBranch] OK

✓ All prepare-commit-msg hooks passed

git rev-parse --show-toplevel
EXIT STATUS: 0
STDOUT: "/Users/cmackenzie1/work/json-faker-lambda\n"
STDERR: ""
git rev-parse --git-dir
EXIT STATUS: 0
STDOUT: ".git\n"
STDERR: ""
git rev-parse --git-common-dir
EXIT STATUS: 0
STDOUT: ".git\n"
STDERR: ""
git config --local --get overcommit.configuration.verifysignatures
EXIT STATUS: 0
STDOUT: "0\n"
STDERR: ""
git config --local --get overcommit.configuration.verifysignatures
EXIT STATUS: 0
STDOUT: "0\n"
STDERR: ""
Running commit-msg hooks
Check subject line................................[SingleLineSubject] OK
Check commit message matches expected pattern.........[MessageFormat] OK
Check for trailing periods in subject................[TrailingPeriod] OK
Check subject capitalization.....................[CapitalizedSubject] OK
Check text width..........................................[TextWidth] OK

✓ All commit-msg hooks passed

git rev-parse --show-toplevel
EXIT STATUS: 0
STDOUT: "/Users/cmackenzie1/work/json-faker-lambda\n"
STDERR: ""
git rev-parse --git-dir
EXIT STATUS: 0
STDOUT: ".git\n"
STDERR: ""
git rev-parse --git-common-dir
EXIT STATUS: 0
STDOUT: ".git\n"
STDERR: ""
git config --local --get overcommit.configuration.verifysignatures
EXIT STATUS: 0
STDOUT: "0\n"
STDERR: ""
git config --local --get overcommit.configuration.verifysignatures
EXIT STATUS: 0
STDOUT: "0\n"
STDERR: ""
✓ No applicable post-commit hooks to run
[PROX-3049-test-event-generation f78c5df] PROX-3049 |  Does not have preceding newline
 1 file changed, 1 insertion(+)

> git log
commit f78c5dfa81aa4226f3700efb182344d03ae4e612 (HEAD -> PROX-3049-test-event-generation)
Author: Cole Mackenzie <cole_mackenzie@intuit.com>
Date:   Wed Feb 19 15:57:29 2020 -0800

    PROX-3049 |  Does not have preceding newline

[cmackenzie1]

@dcluna I am looping you in as you originally implemented the ReplaceBranch hook

[cmackenzie1]

After further investigation it looks like old_contents is set to the value of whatever is provided by -m flag and the usual help comments. Given this file already contains a preceding newline character, the addition of the \n character on L20 is redundant.

[sds]

Thanks for the clear ticket and reproduction steps, @cmackenzie1. Would welcome a pull request fixing this!