-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
syntax: Provide default.yaml as fallback definition #2933
Conversation
1cade4e
to
489d1d7
Compare
I like the idea but I think your default.yaml is doing too much. A lot of the time when I'm editing a file with an unknown type it's going to be plaintext in which case I don't really want special characters or quotes to be highlighted. Thus I'd prefer to only include rules that are extremely unlikely to match in an unrelated context, similar to the Your definition for config categories/sections currently allows newlines which is probably not intended. I would change it to - identifier: "^\\[[^\\[\\]]+\\]$" I would also add the spaces before tabs highlighting from - error: " +\\t+" |
489d1d7
to
2fa818a
Compare
That's something which can be discussed, because it's something tightly coupled to personal preferences. When I edit "unknown" file types then these are quite often configuration files present in the
Yes, you're right. Should have been used as in the nano rule from the beginning.
Done. |
Wondering if that could be addressed with something like this in settings.json:
Maybe not, but you can also always resort to some Lua scripting. |
There is a PR #1897. We might still hope it will be merged some day. If it will, and if we even go ahead and make those |
But then we add for every location again a new settings entry, like path x and then y etc. pp.
...so why not doing more or less the same as Otherwise I expect some more "issues" being opened like the one I try to fix with this feature. 😄 |
I'm not against a default highlighting as such, I was merely commenting on your specific use case of editing various configs files in /etc, for which you might want a richer highlighting than what is suitable for a default template (i.e. for arbitrary plain-text files), as @Andriamanitra pointed out. |
Ah, ok. Then I got you wrong yesterday...sorry for that. Any suggestions what else shall be removed or added?
Then we shouldn't add it from the beginning, am I right? Otherwise it needs to be removed afterwards, in case #1897 is merged. Edit: |
2fa818a
to
8f53bed
Compare
I used your tip for now and set the file type of these files to |
nice, i would have added a screenshot to illustrate tho |
Are there any additional ideas, hints or suggestions to add for the default/unknown definition? |
Just came cross this issue. Would like see default syntax configuration. For example sudo nano /etc/ssh/ssh_config ..etc. |
Overriding the |
8f53bed
to
4b50efb
Compare
break | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about de-duplicating the code? The size of UpdateRules()
is getting scary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I started the refactoring in #3206. This PR will benefit of it too in the moment of necessary rebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My work on #3208 made me realize that such refactoring is probably is not an easy thing to do, unfortunately. The logic in UpdateRules()
is quite subtle, it's not obvious to me how to abstract any important pieces of it into separate functions without loss of functionality and correctness.
Perhaps we should approach this from a different direction: instead of trying to restructure the code, just add some helpers to the syntax parser API, e.g. ParseDefFromFile()
(which would internally just call ParseFile()
and then ParseDef()
), maybe also FindRuntimeFile()
and FindRealRuntimeFile()
or something like that, to make the code of UpdateRules()
at least shorter and a bit easier to read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[...] just add some helpers to the syntax parser API, e.g. ParseDefFromFile() (which would internally just call ParseFile() and then ParseDef()), maybe also FindRuntimeFile() and FindRealRuntimeFile() or something like that, to make the code of UpdateRules() at least shorter and a bit easier to read.
Yep, this will reduce the duplication of load/parses and logs in that way that they aren't spread all over UpdateRules()
.
I will take care of this in the moment #3208 is merged, because then the whole change needs a rebase and I will drop some lines I did.
This is necessary since DoEvent() isn't called in a loop like in the main application, but as one-shot only and a async draw event can lead to ignore the explicit injected events. Additional checks have been added to check the presence of the expected buffers.
4b50efb
to
27beb10
Compare
27beb10
to
82c3d01
Compare
…okup It needs to be processed earliest in the moment no match could be determined.
This will reduce the length of this function and thus improves the readability.
- `findRealRuntimeSyntaxDef()` - `findRuntimeSyntaxDef()` This will reduce the length of this function again and thus improves the readability.
82c3d01
to
6cd39ef
Compare
This will provide a basic rule set which is applied in the moment no known file type can be found or identified. The approach is heavily inspired by
nano
and his default.nanorc. Now it's possible that files without any extension will be highlighted even they aren't explicitly added to a dedicated syntax definition (e.g./etc/fstab
or config files present under/etc/default/
).Closes #2926