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

feat(ui): Improve the new highlighting system #416

Open
Sigmanificient opened this issue Dec 27, 2024 · 5 comments · May be fixed by #422
Open

feat(ui): Improve the new highlighting system #416

Sigmanificient opened this issue Dec 27, 2024 · 5 comments · May be fixed by #422
Labels
help wanted 🙏 Extra attention is needed UI 👔 About user interface

Comments

@Sigmanificient
Copy link
Contributor

Currently, the new highlighter is calibrated to use rust, but it causes issues when rendering Makefile. As you can see comments are highlighted. Moreover, the tool would benefit a lot from a more in-depth colorizing

image

@Sigmanificient
Copy link
Contributor Author

Having a color highlighting similar to nvim capabilities would be really nice:

image

@kyu08
Copy link
Owner

kyu08 commented Dec 29, 2024

@Sigmanificient

Thanks for your feed back! 🚀
At first, I agree your opinion. I'll show why the syntax highlighting is poor after v0.50.0.

I had no choice but poor syntax highlighting because if the language other than ml is specified here, when fzf-make is launched in a neovim terminal, unexpected background highlighting happened.

If you know something to fix this, please let me know.

If the language other than ml is specified here

In this case, make is specified.

In a neovim terminal Not in a neovim terminal
image image

I think this issue need to be solved, so I remain this issue open.

@kyu08 kyu08 added UI 👔 About user interface help wanted 🙏 Extra attention is needed labels Dec 29, 2024
@Sigmanificient
Copy link
Contributor Author

Sigmanificient commented Dec 29, 2024

I also run into the same issue when trying to change the ml extension for mk:

  postPatch = ''
    substituteInPlace src/usecase/tui/ui.rs \
      --replace-fail '("ml")' '("mk")'
  '';

image

Taking a closer look at syntect library, I come to the conclusion that it uses multiples submodules to register highlighters, listed within the testdata directory I believe the Makefile definition comes from this file. After digging a bit more, it turns out that bat is also using the same library 😮, and better, the same definition to parse the Makefile syntax, along a small patch to improve performance. Their approach is very generic, which makes it hard for me to understand what is happening, as I only have a very small background with rust. From what I see, they seem to load binary files containing a compressed version of the highlighter.

I was a sublime text user a while ago, so I have tinkered with making my own syntax, and I remembered that there was also a file to map matched keywords to their respective colors. I wrote a small language definition for the HRM language, take a look at the hrm.sublime-color-scheme file. Maybe there could be a way to provide a custom one to the library?

@Sigmanificient
Copy link
Contributor Author

Sigmanificient commented Dec 29, 2024

let ss = SyntaxSet::load_defaults_newlines();

let mut ts = ThemeSet::load_defaults();
ts.add_from_folder("./").unwrap();
// dbg!(&ts.themes);

let theme = &mut ts.themes["OneHalfDark"].clone();
let syntax = ss.find_syntax_by_extension("mk").unwrap();

Using this bit of code, I was able to successfully load a OneHalfDark.tmTheme (an equivalent to a .sublime-color-scheme, using XML instead of YAML) I have borrowed for my experiment 😄

image

I made a branch on my fork in case you would need to test it:
https://github.com/Sigmanificient/fzf-make/tree/proof-of-concept

@kyu08
Copy link
Owner

kyu08 commented Jan 2, 2025

I made a branch on my fork in case you would need to test it:
https://github.com/Sigmanificient/fzf-make/tree/proof-of-concept

@Sigmanificient
Wow! Thanks! This looks perfect even within neovim's terminal!

Could you please submit as PR? I'll review it asap.

image

@Sigmanificient Sigmanificient linked a pull request Jan 3, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted 🙏 Extra attention is needed UI 👔 About user interface
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants