c4urself / bump2version

Version-bump your software with a single command
https://pypi.python.org/pypi/bump2version
MIT License
1.05k stars 134 forks source link

Change the use of "→" to "->" in the default commit message #192

Open markmacode opened 3 years ago

markmacode commented 3 years ago

Issue 1 (ease of use)

It is easier to work with 2 characters that I can type on my keyboard, than to work with a Unicode character that I will have to copy and paste to be able to use. There may be situations where someone will use a script that needs to reference the commit messages of a bumped version, where string splitting may be done based on the separator. Separating with -> should be much easier to work with it in a scenario like that.

Issue 2 (Git Bash support)

Another reason (the main reason for myself) is that the current character is error prone when using Git Bash on Windows. When I am on Windows I tend to use Git Bash as my terminal when I know I will not need to use WSL2. When using bump2version -h in Git Bash, I get the following error, and the help description does not get displayed at all:

  File "C:\Program Files\Python38\lib\encodings\cp1252.py", line 19, in encode
    return codecs.charmap_encode(input,self.errors,encoding_table)[0]
UnicodeEncodeError: 'charmap' codec can't encode character '\u2192' in position 3036: character maps to <undefined>

The \u2192 character it is referencing is the Unicode for the right arrow used in the commit message.

My thoughts

Granted this is a bug relating the default install for Git Bash, you could argue this project is not responsible for it. Although if someone using Git Bash gets bump2version for the first time and sees this error when they first try to use it, I can imagine that they will likely dismiss bump2version and uninstall it, looking for an alternative. And I feel like the use of the character is only done for the sake of it looking good, which I do not think is a good reason to use it over the issues I outlined above.

I would be happy to take this on and update the code to change this, if people agree on this change.

florisla commented 3 years ago

I'm slightly in favor of changing this. Only slightly, because such a change might break some user's workflows.

I'm using Windows a lot, and all my config files have a custom message due to reason nr 2) above.

Unless 2) might be fixable somehow.

Note: I don't think we should change this for Windows only, because then a single repository gets different commit messages depending on which OS you're committing from.

Edit: Found this link explaining what's preventing Python from printing proper Unicode characters. https://stackoverflow.com/questions/45660817/python-print-unicode-string-via-git-bash-gets-unicodeencodeerror

markmacode commented 3 years ago

such a change might break some user's workflows.

Possibly, a change like this might only be reasonable in version 1.1.0. Seeing a real world scenario where someone depends on the current default commit message would help. Because right now it is just speculation of possible issues with changing it.

Unless 2) might be fixable somehow.

I played around with some potential fixes, nothing allowed the to appear.

I don't think we should change this for Windows only, because then a single repository gets different commit messages depending on which OS you're committing from.

I agree - Also bump2version probably should not take on the responsibility of encoding issues of different platforms due to a single character.


When I was scrolling through the tests, I noticed that this problem was actually addressed 2 years ago test_cli.py:192. If we use -> by default in the commit messages, we can remove the need for any OS conditions in the code. And it will allow bump2version to be usable on more platforms.

Also when exploring this issue some more, I discovered that --dry-run does not display the arrow either, it displays \u2192 instead. This is only for Git Bash.

c4urself commented 3 years ago

Hmm I agree with @florisla -- rethinking from day 1 it would've been better to go with -> or to. I'm hesitant to add this if it's common to rely on it / breaks the user. That said, it's a terrible "api" to depend on -- for example crawling through commit history to figure out when a bump happened by checking for "→" -- versus checking for tags... Additionally, folks can still override via config.

markmacode commented 3 years ago

@c4urself You are right, it is a terrible "api" to depend on, and my mentioning of it as Issue 1 was purely speculation. And I guess the main issue here is, do we consider the commit message to be part of bump2version's API?

folks can still override via config

I can see this as a reason to keep it. But I can also see this as a reason for it to be changed, as people who do depend on the default commit message for whatever reason, can change their config if they update bump2version to a new release with this change.

I guess I have a worse feeling thinking about people disregarding bump2version if they see this error the first time they try to use it. Compared to people depending on the default commit message, needing to change their config or workflow (which is optional, we would not force them to update their bump2version to a new release).