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

scrolling down on commit list with 'Invalid collation character' errors slowly garbles screen #39

Closed
flokli opened this issue Feb 8, 2018 · 10 comments
Labels

Comments

@flokli
Copy link

flokli commented Feb 8, 2018

grv-garble

I invoked grv (0.1.2) on a git checkout of the linux tree (at b89e32ccd1be92a3643df3908d3026b09e271616), tabbed to the git log, and moved the cursor down 13 times to commit babcbbc.

There seems to be some garbled encoding somewhere in this commit (although I couldn't find it using a git show), grv doesn't update the commit message and keeps showing 8e44e66 in the message pane, but the cursor advances, and an error message Invalid collation character is shown on the bottom of the screen.

Moving down further, space to show more error messages gets allocated (as the regular 'window height' seems to get reduced), but the messages are not shown, and the window rendering gets garbled.

The collation error seems to be related to my locale (en_US.UTF-8), and is gone when invoking with LANG=C.

@rgburke rgburke added the bug label Feb 8, 2018
@rgburke
Copy link
Owner

rgburke commented Feb 8, 2018

Thanks for reporting this. There are two parts to this issue:

  • The collation error
  • The screen becoming garbled

The second part of the issue is similar to what was reported in #25. I suspect that displaying the error window causes the garbled appearance of the screen in some way. So I wanted to check, when you run grv with LANG=C was the screen no longer garbled?

@flokli
Copy link
Author

flokli commented Feb 9, 2018

Sorry for being unspecific :-)

With LANG=C, no screen garbling and no collation error message appears.

@flokli
Copy link
Author

flokli commented Feb 9, 2018

Yeah, garbling, looks pretty similar to that shown in #25, although it's locale-related for me and the error message is different.

@rgburke
Copy link
Owner

rgburke commented Feb 10, 2018

Thanks for confirming the behaviour. The issue with the screen getting garbled is now fixed on master.

I've had a brief look into the collation error and it appears that it is being returned by libgit2. I will need to look into this more and possibly raise an issue against libgit2.

@flokli
Copy link
Author

flokli commented Feb 12, 2018

I can confirm the screen garbling itself is fixed. Error messages (max 5) are now displayed properly as intended.
Did you by any chance already take a look into libgit2?

@rgburke
Copy link
Owner

rgburke commented Feb 12, 2018

Yes, I've had a chance to look into this a bit further. The issue appears to be locale related. GRV calls setlocale(LC_ALL, ""); on initialisation as this is standard practice for ncurses programs and allows them to correctly display unicode characters.
However doing so causes libgit2 to return the "Invalid collation character" error when loading certain diffs. Based on the similar issue nodegit/nodegit#1097 it appears that regcomp fails with this error when running a series of regex's against the diff content to determine if a line matches a function declaration (which gets shown in the hunk header).
The other strange aspect is that this issue only seems to appear on the Linux repo. Other repo's which have unicode characters (in author names, commit messages, etc) don't manifest the same bug.
I've raised libgit2/libgit2#4528 against libgit2 for this issue. At the moment I can't see a workaround for this issue other than to set LC_ALL=C when viewing the Linux repo.

@rgburke
Copy link
Owner

rgburke commented Jun 22, 2018

A workaround has been put in place and this issue should be resolved on master. It doesn't seem like the issue raised against libgit2 will be easy to resolve. So GRV now shells out to git show in order to generate the diff when the Invalid collation character error is detected.

@flokli
Copy link
Author

flokli commented Jun 23, 2018

@rgburke thanks for keeping track of that.
As a heads-up, added a comment to the introduced run time dependency on the git binary here.

@rgburke
Copy link
Owner

rgburke commented Jun 24, 2018

@flokli Based on your comments on the commit, a git-binary-file-path config variable has been added that allows a user to specify the file path to the git binary. When GRV detects that it needs to fall back to git it will attempt to run git version (using git or the value of git-binary-file-path if set). If this fails it will prompt the user with an error message informing them to set git-binary-file-path.

For users with git in their $PATH (which is probably most) this won't be an issue. Those users without git in their $PATH will be explicitly prompted to set git-binary-file-path. Hopefully this solution is sufficient to handle the case where git is not in $PATH.

@rgburke
Copy link
Owner

rgburke commented Jan 9, 2019

Closing this as a workaround has been in place for a while now.

@rgburke rgburke closed this as completed Jan 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants