Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Commit prefix is added even when there is no regex match #3695

Closed
phaze-ZA opened this issue Jun 27, 2024 · 1 comment
Closed

Commit prefix is added even when there is no regex match #3695

phaze-ZA opened this issue Jun 27, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@phaze-ZA
Copy link
Contributor

phaze-ZA commented Jun 27, 2024

Describe the bug
When using the commitPrefix(es) pattern and replace config, the expectation is that any non-match should not result in the replace being returned. For e.g. if I have pattern: "^(JIRA-\\d+)-.*" replace: "$1: " and my branch name is master then the return should just be an empty string, not master.

To Reproduce
Steps to reproduce the behavior:

  1. Set commitPrefix pattern and replace in config.yaml
  2. Create a branch that doesn't match your pattern
  3. Start a commit
  4. Notice the branch name is in the commit box even though there was no match

Expected behavior
There should be no commit prefix if there was no regex match

Screenshots
image

image

Version info:
commit=6fcb7eb8bb616c170506312870b3bf15f3dbe37c, build date=2024-05-19T10:15:28Z, build source=binaryRelease, version=0.42.0, os=darwin, arch=arm64, git version=2.39.3 (Apple Git-146)

git version=2.39.3 (Apple Git-146)

Additional context
I've set up a couple of Go Playgrounds to demonstrate more or less the expected behaviour.
Expected: https://go.dev/play/p/se9Sex7v9YG
Current: https://go.dev/play/p/6Kt1NM9ZA2t

Just change the branch_name variable to see your thingy things

This would be the area that requires changing:

func (self *WorkingTreeHelper) HandleCommitPress() error {
message := self.c.Contexts().CommitMessage.GetPreservedMessage()
if message == "" {
commitPrefixConfig := self.commitPrefixConfigForRepo()
if commitPrefixConfig != nil {
prefixPattern := commitPrefixConfig.Pattern
prefixReplace := commitPrefixConfig.Replace
rgx, err := regexp.Compile(prefixPattern)
if err != nil {
return fmt.Errorf("%s: %s", self.c.Tr.CommitPrefixPatternError, err.Error())
}
prefix := rgx.ReplaceAllString(self.refHelper.GetCheckedOutRef().Name, prefixReplace)
message = prefix
}
}
return self.HandleCommitPressWithMessage(message)
}

@phaze-ZA phaze-ZA added the bug Something isn't working label Jun 27, 2024
@phaze-ZA phaze-ZA changed the title Use MatchString before ReplaceAllString in HandleCommitPress Commit prefix is added even when there is no regex match Jul 1, 2024
stefanhaller added a commit that referenced this issue Jul 10, 2024
- **PR Description**
Currently if a branch name does not match a regex pattern defined in the
config.yaml (commitPrefix/es) the commit message box is populated with
the branch name as is - this does not match expectations. A prefix
should only be added if there is a match on the regex pattern.

This PR seeks to change that by checking for a match before calling
ReplaceAllString - see Issue #3695

- **Please check if the PR fulfills these requirements**

* [x] Cheatsheets are up-to-date (run `go generate ./...`)
* [x] Code has been formatted (see
[here](https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#code-formatting))
* [x] Tests have been added/updated (see
[here](https://github.com/jesseduffield/lazygit/blob/master/pkg/integration/README.md)
for the integration test guide)
* [-] Text is internationalised (see
[here](https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#internationalisation))
* [-] Docs have been updated if necessary
* [x] You've read through your own file changes for silly mistakes etc
@stefanhaller
Copy link
Collaborator

Fixed by #3703.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants