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

Fix alignment in blame mode when author name contains unicode accent modifiers #1456

Merged
merged 1 commit into from Jun 23, 2023

Conversation

hpwxf
Copy link
Contributor

@hpwxf hpwxf commented Jun 23, 2023

This PR fixes an issue found when an author name contains Unicode modifiers.
For example, the following blame entries:

95d7e884 (azerty      2022-06-23 11:02:48 +0200  1) 
723893a9 (Jean Brauns 2022-06-30 15:47:48 +0200  3)
e9f57a43 (Pascal Have 2023-05-23 17:16:54 +0200  4) # without unicode accent => good alignement
723893a9 (Jean Brauns 2022-06-30 15:47:48 +0200  5)
723893a9 (Jean Brauns 2022-06-30 15:47:48 +0200  6)
95d7e884 (azerty      2022-06-23 11:02:48 +0200  7)
95d7e884 (azerty      2022-06-23 11:02:48 +0200  8)
15b8082e (Jean Brauns 2023-06-22 22:54:06 +0200  9)
e9f57a43 (Pascal Havé 2023-05-23 17:16:54 +0200 10) # with unicode accent => bad alignement
e9f57a43 (Pascal Havé 2023-05-23 17:16:54 +0200 11) # missing space in the padding
95d7e884 (azerty      2022-06-23 11:02:48 +0200 12)
c3d9a790 (azerty      2022-08-30 11:32:58 +0200 13)
e9f57a43 (Pascal Havé 2023-05-23 17:16:54 +0200 14) # by using this example, if the error doesn't occur,
e9f57a43 (Pascal Havé 2023-05-23 17:16:54 +0200 15) # you're probably using an ASCII accent not a Unicode accent modifier
95d7e884 (azerty      2022-06-23 11:02:48 +0200 16)
15b8082e (Jean Brauns 2023-06-22 22:54:06 +0200 17)

delta < file.blame produces (master v0.16.5)
Capture d’écran 2023-06-23 à 14 20 20

It only uses an already embedded crate (unicode_width).

It includes a test case to check the number of trailing spaces with an author's name with or without an accent.

PS: with the suggested fix:
Capture d’écran 2023-06-23 à 14 28 25

NB: this was the shortest patch I can do. It could be more efficient by only processing extra width for the author string (not all strings as it is in the current patch). However, this alternative is possible by setting an extra width inside field (the type will be (Cow<str>, usize) with a default value of 0 except author: hpwxf@dc38ca9

@dandavison
Copy link
Owner

Thanks! I think this version LGTM. (Is there any way someone could get non-ASCII characters into the timestamp field in a way that varies between lines?)

@dandavison dandavison merged commit ff5fd06 into dandavison:master Jun 23, 2023
12 checks passed
@hpwxf
Copy link
Contributor Author

hpwxf commented Jun 23, 2023

@dandavison about non-ASCII characters into timestamps, I don't think so; that's why I also linked an alternative solution (less local changes): hpwxf@dc38ca9. If you prefer this one, I can submit it.

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.

None yet

2 participants