Allow syntax highlighting to be disabled #2053

Mazurel · 2022-04-09T17:41:09Z

This PR resolves issue #338.

It adds a possibility for syntax highlighting to be disabled for some document.
Such occurrence happens automatically for "big" files and can also be triggered artificially by a command from command palette.
Running the command toggles the syntax highlighting on/off, so it can be easily re-enabled if needed.

I have assumed that "big" file is more than or equal to 128 Megabytes in size. After this size, documents tend to lag, but this number if up for debate.

dead10ck · 2022-04-09T18:21:11Z

helix-view/src/document.rs

+                }
+            } else {
+                // If there is some syntax and it should be disabled, disable it.
+                self.syntax = None;


If I'm understanding this right, this isn't really just disabling highlighting, it's disabling any and all syntax tree parsing. Is the issue of slow speed specifically due to highlight rendering, or is it caused by having such a large syntax tree loaded into memory at once? Disabling the syntax parsing will also disable text objects, auto complete, etc.

If it's specifically caused by highlighting, then we may want to disable highlighting without disabling all of the syntax tree parsing. And if we do want to disable syntax parsing altogether, then I think we should rename the command and struct fields something like enable_tree_sitter or something.

Yeah, you are completely right, this is more about disabling syntax parsing then highlighting. Highlighting (or rendering) by itself is not an issue, the parser is. I have experimented with the parser, but I have not found any better solution than disabling it completely.

I will rename commands and struct fields to names that are more accurate, as you have suggested.

helix-view/src/document.rs

archseer · 2022-04-10T02:11:53Z

helix-term/src/commands.rs

@@ -414,7 +414,8 @@ impl MappableCommand {
        decrement, "Decrement",
        record_macro, "Record macro",
        replay_macro, "Replay macro",
-        command_palette, "Open command palette",
+        command_palette, "Open command pallete",
+        toggle_syntax_highlighting, "Toggle syntax highlighting for current document",


We don't need a separate command, it should be the equivalent of setting the language to none/unset via the set language command

Hmm though yeah this would disable lsp and other integrations too

archseer · 2022-04-10T02:12:19Z

helix-view/src/document.rs

@@ -101,6 +105,8 @@ pub struct Document {
    syntax: Option<Syntax>,
    /// Corresponding language scope name. Usually `source.<lang>`.
    pub(crate) language: Option<Arc<LanguageConfiguration>>,
+    /// Sets if syntax highlighting should be used or not
+    pub highlight_syntax: bool,


Should be removed and set the language to None

Currently, it is not an issue, but I think that setting language to None when disabling parser, may cause some confusion to someone in the future. Language by itself is not changing/non-existent, it is just not parsed.

Mazurel · 2022-05-15T15:56:29Z

Sorry for leaving this PR with no response for so long, I have unexpectedly had way more personal work to do, than I have expected. I will try to be more active from now on.

helix-term/src/commands.rs

helix-view/src/document.rs

Mazurel · 2022-05-29T09:29:40Z

I have implemented prompt when syntax parsing is disabled, as suggested above. This required some code modifications, so I am waiting for responses, as I have probably made some rookie Rust mistakes.

helix-term/src/commands.rs

pickfire · 2022-07-07T14:58:27Z

helix-view/src/document.rs

+
+    pub document_events: (
+        UnboundedSender<DocumentEvent>,
+        UnboundedReceiver<DocumentEvent>,
+    ),


Do we need this? Not sure if possible but I think it is better to just look at enable_syntax, or just trigger a redraw when user set the syntax option.

The way I understand the architecture of this project, the other possibility would be to check some flag in the idle event (in the main tokio loop). This solution however doesn't seem that elegant.

I also think that it is a matter of time, when there will be a need for sending event from document to the rendering loop.

tgharib · 2022-08-13T14:59:19Z

I have assumed that "big" file is more than or equal to 128 Megabytes in size. After this size, documents tend to lag, but this number if up for debate.

I hope this will be configurable in the future. "Big" is dependent on user tolerance for lag, CPU power and the language/treesitter implementation.

- Automatically disable syntax highlighting on big files. - Add command to manually toggle syntax highlighting on and off.

pickfire · 2022-08-28T00:48:10Z

helix-term/src/commands/typed.rs

+fn toggle_syntax(
+    cx: &mut compositor::Context,
+    args: &[Cow<str>],
+    _event: PromptEvent,
+) -> anyhow::Result<()> {


This doesn't handle PromptEvent correctly, it is also toggling on preview, so like :toggle-syntax off<esc> still change the config, it should do syntax toggling correctly and ignore completion event, since preview isn't very useful for toggling syntax.

pickfire · 2022-08-28T00:48:39Z

helix-term/src/commands.rs

@@ -431,7 +431,7 @@ impl MappableCommand {
        decrement, "Decrement item under cursor",
        record_macro, "Record macro",
        replay_macro, "Replay macro",
-        command_palette, "Open command palette",
+        command_palette, "Open command pallete",


Shouldn't this be palette like the command?

https://www.merriam-webster.com/dictionary/palette is the correct spelling

pickfire · 2022-08-28T00:49:51Z

helix-view/src/document.rs

@@ -634,6 +654,15 @@ impl Document {
        // and error out when document is saved
        self.path = path;

+        if self.text.len_bytes() > BIG_FILE_THRESHOLD {
+            self.enable_syntax = false;


There should be a message saying that syntax is disabled or maybe some signs, or otherwise the user might find it surprising.

pascalkuthe · 2024-04-13T23:15:45Z

closing out this PR as its very stale. I also don't see us moving forward in this direction. We have an automatic timeout now for treesitter parsers which covers most related cases. Thank you for contributing!

neuromagus · 2024-04-16T22:26:02Z

What to do with those programming languages where TS grammars do not support the latest language changes? For example - csharp grammar or tsx/typescript? Or parser errors with a new version?

I want to use Helix by turning off this module while the developers fix bugs or make changes. Or are you involved in supporting grammars?

the-mikedavis · 2024-04-17T13:57:51Z

I think it would make sense to respect the use-grammars setting when loading grammars https://docs.helix-editor.com/languages.html#choosing-grammars. Currently it's only used when fetching/building grammars but I think it would be less surprising if setting use-grammars.except = ["c"] stopped the C grammar from loading for example. That should fit your use-case if I understand correctly.

neuromagus · 2024-04-17T15:27:57Z

Currently it's only used when fetching/building grammars but I think it would be less surprising if setting use-grammars.except = ["c"] stopped the C grammar from loading for example.

This is solution.

dead10ck requested changes Apr 9, 2022

View reviewed changes

sudormrfbin reviewed Apr 9, 2022

View reviewed changes

helix-view/src/document.rs Outdated Show resolved Hide resolved

kirawi reviewed Apr 9, 2022

View reviewed changes

helix-view/src/document.rs Outdated Show resolved Hide resolved

archseer requested changes Apr 10, 2022

View reviewed changes

Mazurel force-pushed the master branch from f0b671c to 9725f9e Compare May 15, 2022 15:54

pickfire reviewed May 16, 2022

View reviewed changes

helix-term/src/commands.rs Outdated Show resolved Hide resolved

pickfire reviewed May 16, 2022

View reviewed changes

helix-view/src/document.rs Outdated Show resolved Hide resolved

the-mikedavis added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 18, 2022

the-mikedavis removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 27, 2022

pickfire reviewed Jul 7, 2022

View reviewed changes

helix-term/src/commands.rs Outdated Show resolved Hide resolved

pickfire reviewed Jul 7, 2022

View reviewed changes

Mazurel added 7 commits August 15, 2022 16:45

Implement disabling of the tree-sitter

a42601e

- Automatically disable syntax highlighting on big files. - Add command to manually toggle syntax highlighting on and off.

Apply feedback from Github reviews

2299487

Add event communication with document

dc82b9e

Implement prompt when syntax is automatically disabled

45f4047

Rename toggle_syntax_parser to toggle_syntax

e6be2ca

Add clippy suggestions

9a137dd

Improved toggle-syntax command

ab75f96

Mazurel force-pushed the master branch from dc28874 to ab75f96 Compare August 15, 2022 15:01

pickfire reviewed Aug 28, 2022

View reviewed changes

kirawi added A-helix-term Area: Helix term improvements S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 13, 2022

kirawi added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 24, 2022

pascalkuthe closed this Apr 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow syntax highlighting to be disabled #2053

Allow syntax highlighting to be disabled #2053

Mazurel commented Apr 9, 2022

dead10ck Apr 9, 2022 •

edited

Loading

Mazurel May 15, 2022

archseer Apr 10, 2022

archseer Apr 10, 2022

archseer Apr 10, 2022

Mazurel May 15, 2022

Mazurel commented May 15, 2022

Mazurel commented May 29, 2022

pickfire Jul 7, 2022

Mazurel Aug 14, 2022

tgharib commented Aug 13, 2022

pickfire Aug 28, 2022

pickfire Aug 28, 2022

the-mikedavis Aug 28, 2022

pickfire Aug 28, 2022

pascalkuthe commented Apr 13, 2024

neuromagus commented Apr 16, 2024 •

edited

Loading

the-mikedavis commented Apr 17, 2024

neuromagus commented Apr 17, 2024

Allow syntax highlighting to be disabled #2053

Allow syntax highlighting to be disabled #2053

Conversation

Mazurel commented Apr 9, 2022

dead10ck Apr 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mazurel commented May 15, 2022

Mazurel commented May 29, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tgharib commented Aug 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pascalkuthe commented Apr 13, 2024

neuromagus commented Apr 16, 2024 • edited Loading

the-mikedavis commented Apr 17, 2024

neuromagus commented Apr 17, 2024

dead10ck Apr 9, 2022 •

edited

Loading

neuromagus commented Apr 16, 2024 •

edited

Loading