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

Highlights are based on the literal syntax only, hence often incorrect #66

Closed
wookayin opened this issue Oct 12, 2021 · 6 comments
Closed
Labels
feedback wanted Feedback wanted from the community

Comments

@wookayin
Copy link

wookayin commented Oct 12, 2021

image

Highlights in vim-flog buffers are based on the syntax only rather than its structure (as per the git log format), which can be incorrect if commit messages contain some characters as shown in the above screenshot. Although the commit messages in the screenshot are written a bit artificially, I can see similar issues on several in-the-wild git repositories.

For example:

  • [ alphanumericwords ] : recognized as commit hash
  • ( ...something ... ) : recognized as refspec
  • { ...something ... } : recognized as authors

So any parenthesis, brackets, braces in a commit message will be recognized by flog. Or, if the format string doesn't wrap the commit hash (%h) with [...], it won't be highlighted as flogHash. Similar things can be said for flogAuthor without {...}, etc.

Would it be possible to improve syntax for the highlights so that it could be agnostic to the specific git log format used by Flog (i.e., the -format args)?

@rbong
Copy link
Owner

rbong commented Oct 13, 2021

Would it be possible to improve syntax for the highlights so that it could be agnostic to the specific git log format used by Flog (i.e., the -format args)?

To properly support both strictly correct highlighting which works with custom log formats, Flog would have to implement a parser for the -format command and dynamically generate highlight groups.

I think the added complexity would not be worth it, even if I were not trying to keep Flog light, and there could be a performance hit as well. The simple highlight groups are the tradeoff for custom log formats.

One thing you could do is use the AnsiEsc plugin with Flog. This will use shell highlighting in Flog, however there will be a performance hit, and you will have to use color items (ex. %CredThis text will appear in red%Creset) in your log format. Instructions here.

There has been some interest as well in adding an option to insert hidden characters into the format to improve highlight groups. However, you would have to include these characters in your output format as well. Let me know what you think of this option.

Finally I have made some syntax fixes based on your example. This eliminates ref highlighting sometimes when it can, and it eliminates a bug as well.
You can see it helps but there are still issues.

syntax fixes

I know none of this is exactly what you want to see, but I hope it helps.

@wookayin
Copy link
Author

Thank you @rbong for the detailed and fast response. I understand this would be tricky and difficult to implement. Given that the content of commit logs are not generated by the plugin but by git log ..., we do not have much thing under control. Now I think that, without knowing the format string (technically, even if the plugin knows it), there will be ambiguity in parsing so there will be no perfect/ideal syntax-based approach to have an always correct highlighting.

That said, using AnsiEsc.vim sounds like a good workaround for this. The approach of #64 also sounds interesting.

@rbong rbong added the unlikely to fix Issues which we are likely unable to fix label Jan 26, 2023
@rbong
Copy link
Owner

rbong commented Jan 26, 2023

The only viable true solution right now is to use hidden characters.

Because I am not very interested in this feature, I can't think of a nice user experience, and the last time I benchmarked inserting a ton of hidden characters into the output it was not performant, I'm unlikely to fix this issue unless if there's a lot of support.

Also AnsiEsc support has been removed from v2 since the biggest reason it was implemented (graph branch color) has been resolved. If there is interest in bringing it back, I may just implement Ansi code highlighting myself since it would be much more controlled and performant.

@rbong rbong added the feedback wanted Feedback wanted from the community label Jan 26, 2023
@rbong rbong removed the unlikely to fix Issues which we are likely unable to fix label Aug 19, 2024
@rbong
Copy link
Owner

rbong commented Aug 19, 2024

Performance is no longer such an issue, but only in Neovim, so I will implement this in v3. The v3 branch is available for testing, but currently only has more accurate branch highlighting, not commit subject highlighting. You can find more details here: #135

@rbong
Copy link
Owner

rbong commented Aug 25, 2024

I'm realizing re-implementing ANSI escape codes is not the right way to implement this feature.

  • ANSI escape codes are used for diffs so any customization to red/green escape codes will also impact the way diffs look.
  • In gvim/neovim, the default colors would be very basic and require custom colorschemes (can't fall back to the customized terminal colors).
  • It is obviously not very intuitive to customize colors like "red", "green", "blue", etc. when trying to customize things like dates, hashes, author names.
  • Ref names would be left with a single color instead of having different colors for the different parts (parentheses, remote, tags, etc.)

The solution will be better but may still include hidden characters. Still thinking about the correct interface.

@rbong
Copy link
Owner

rbong commented Aug 27, 2024

I added accurate commit highlighting to v3. It works in both Vim and Neovim. It's off by default. It uses custom concealed escape codes.

Enable it with:

let g:flog_enable_dynamic_commit_hl = 1

Pros:

  • No false matches for commit body highlighting
  • More flexible commit body highlighting
  • Don't have to put special characters in your commit format
  • About the same performance on scroll as v2 with it on, except without the screen tearing of v2
  • Much better performance on scroll than AnsiEsc.vim when combined with v3 dynamic graph highlights
  • Automatically works with any log format
  • No initialization time performance degredation

Cons:

  • Performance degradation on scroll compared to baseline v3 with the setting off (because of concealed characters)
  • Concealed characters in the output can be awkward (but I will add yank targets for commit subject/body that will sanitize concealed characters)

For me, because I am happy with the performance on scroll in v2 anyways on my machine (PLEASE post an issue with your specs if you are not), I am going with this implementation, but leaving it off by default because of the performance issue.

If you have any feedback, issues, or questions, please post in the v3 issue or start a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feedback wanted Feedback wanted from the community
Projects
None yet
Development

No branches or pull requests

2 participants