Skip to content

Conversation

@FreddyFunk
Copy link

PR Description

Please check if the PR fulfills these requirements

  • Cheatsheets are up-to-date (run go generate ./...)
  • Code has been formatted (see here)
  • Tests have been added/updated (see here for the integration test guide)
  • Text is internationalised (see here)
  • If a new UserConfig entry was added, make sure it can be hot-reloaded (see here)
  • Docs have been updated if necessary
  • You've read through your own file changes for silly mistakes etc

Replace space characters with minus charcater when naming new branch names.

@stefanhaller
Copy link
Collaborator

Lazygit has this functionality already, it just doesn't show the dashes directly while typing. It replaces spaces with dashes after you hit enter. Since there is no PR description, I couldn't tell whether you are aware of this and want to improve on it with this PR.

Personally I think the current behavior is fine; I find it a bit weird that you press a key and a different one appears.

@FreddyFunk
Copy link
Author

I find it a bit weird that you press a key and a different one appears.

For me it is the other way around. If enter a branch name and then I press enter, the name gets silently and automatically changed. I would rather see what the final branch name is going to be while I input the branch name.

I am used to this behavior by other IDEs like the ones from JetBrains.

@stefanhaller
Copy link
Collaborator

Ok, but now I'm not sure how to resolve this. Anybody else listening who wants to chime in with an opinion?

@ChrisMcD1
Copy link
Contributor

I don't find myself typing spaces into branch names ever really, but I think it would be fine to have them replaced at that point.

I don't love where this implementation chooses to make that change though. It feels like a very generic location to have the behavior of a very specific situation, which really is just branch creation

@stefanhaller
Copy link
Collaborator

I don't love where this implementation chooses to make that change though. It feels like a very generic location to have the behavior of a very specific situation, which really is just branch creation

I had the same thought, but I didn't even start commenting on the implementation yet, because I'm still skeptical about the behavior. If we were to make this change, we should probably replace the ReplaceSpacesWithDashes bool with some sort of InputFilter func that allows clients to change what is typed in whatever way they want. This would include replacing a character with a different one, filtering out characters altogether, or making whatever other changes (might require access to what was typed already). Designing it properly is a bit tricky, because depending on the nature of the change there might be consequences for the cursor position. But honestly, I don't feel it's worth spending more time on designing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants