These are two improvements of the comments discussed in #3413:
1. The indented `#command_example();` in ```ini # set to false if a comment character/string should start at column 0 of a line, true uses any # indentation of the line, e.g. setting to true causes the following on pressing CTRL+d #command_example(); # setting to false would generate this # command_example(); # This setting works only for single line comments comment_use_indent=true ``` isn't a valid conf file line as `#` should be placed at the beginning of the line. This makes Scintilla to render `#command_example();` as if it weren't a comment. This patch changes this to ```ini # set to false if a comment character/string should start at column 0 of a line, true uses any # indentation of the line, e.g. setting to true causes the following on pressing CTRL+d # #command_example(); # setting to false would generate this # # command_example(); # This setting works only for single line comments ```
2. The patch replaces the not-very-English-sounding ```ini # the following characters are these which a "word" can contains, see documentation #wordchars=_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 ``` with the following description from the documentation: ```ini # these characters define word boundaries when making selections and searching # using word matching options #wordchars=_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 ``` You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/3428
-- Commit Summary --
* Place # at the beginning of the line with the example in filetype conf files * Reword "wordchars" description using the description from the manual
-- File Changes --
M data/filedefs/filetypes.Arduino.conf (4) M data/filedefs/filetypes.CUDA.conf (7) M data/filedefs/filetypes.Clojure.conf (7) M data/filedefs/filetypes.Genie.conf (3) M data/filedefs/filetypes.Graphviz.conf (7) M data/filedefs/filetypes.Scala.conf (3) M data/filedefs/filetypes.Swift.conf (3) M data/filedefs/filetypes.TypeScript.conf (7) M data/filedefs/filetypes.abaqus (7) M data/filedefs/filetypes.actionscript (7) M data/filedefs/filetypes.ada (7) M data/filedefs/filetypes.asm (7) M data/filedefs/filetypes.batch (7) M data/filedefs/filetypes.c (7) M data/filedefs/filetypes.caml (7) M data/filedefs/filetypes.cmake (7) M data/filedefs/filetypes.cobol (7) M data/filedefs/filetypes.conf (7) M data/filedefs/filetypes.cpp (7) M data/filedefs/filetypes.cs (7) M data/filedefs/filetypes.css (7) M data/filedefs/filetypes.d (7) M data/filedefs/filetypes.diff (3) M data/filedefs/filetypes.docbook (7) M data/filedefs/filetypes.erlang (7) M data/filedefs/filetypes.f77 (7) M data/filedefs/filetypes.forth (7) M data/filedefs/filetypes.fortran (7) M data/filedefs/filetypes.freebasic (7) M data/filedefs/filetypes.gdscript (4) M data/filedefs/filetypes.glsl (7) M data/filedefs/filetypes.go (7) M data/filedefs/filetypes.haskell (7) M data/filedefs/filetypes.haxe (7) M data/filedefs/filetypes.html (7) M data/filedefs/filetypes.java (7) M data/filedefs/filetypes.javascript (7) M data/filedefs/filetypes.latex (7) M data/filedefs/filetypes.lisp (7) M data/filedefs/filetypes.lua (7) M data/filedefs/filetypes.makefile (7) M data/filedefs/filetypes.matlab (7) M data/filedefs/filetypes.nsis (7) M data/filedefs/filetypes.objectivec (7) M data/filedefs/filetypes.pascal (7) M data/filedefs/filetypes.perl (7) M data/filedefs/filetypes.php (7) M data/filedefs/filetypes.po (7) M data/filedefs/filetypes.powershell (7) M data/filedefs/filetypes.python.in (7) M data/filedefs/filetypes.r (7) M data/filedefs/filetypes.restructuredtext (7) M data/filedefs/filetypes.ruby (7) M data/filedefs/filetypes.rust (4) M data/filedefs/filetypes.sh (9) M data/filedefs/filetypes.smalltalk (7) M data/filedefs/filetypes.sql (7) M data/filedefs/filetypes.tcl (7) M data/filedefs/filetypes.vala (7) M data/filedefs/filetypes.verilog (7) M data/filedefs/filetypes.vhdl (7) M data/filedefs/filetypes.xml (7) M data/filedefs/filetypes.yaml (7)
-- Patch Links --
https://github.com/geany/geany/pull/3428.patch https://github.com/geany/geany/pull/3428.diff
- The indented `#command_example();` in
[…]
isn't a valid conf file line as `#` should be placed at the beginning of the line. This makes Scintilla to render `#command_example();` as if it weren't a comment. This patch changes this to
Actually it [is valid for our purpose at least](https://gitlab.gnome.org/GNOME/glib/-/blob/main/glib/gkeyfile.c#L1277).
This makes Scintilla to render `#command_example();` as if it weren't a comment.
Yeah that's not very nice indeed, so it'd be indeed good to stick to what that lexers accepts as a comment. However, we explicitly set `lexer.props.allow.initial.spaces=0` in *filetypes.conf*, which we probably shouldn't… setting it to `1` also fixes the highlighting in this file.
For history, the property originates from 00bc23cec4d86d5e18752dc38034978ac18f9645, which commit message confuses me, as I understand it as meaning the exact contrary to what it actually does…
It's however worth noting that the [tag parser ignores lines starting with a space regardless of what comes after](https://github.com/geany/geany/blob/master/ctags/parsers/iniconf.c#L127), e.g. ` key=value` isn't parsed as a key-value pair.
---
I'm not saying we shouldn't apply your changes here, they *do* look better to me. I'm just trying to be (too) thorough.
@b4n
Actually it [is valid for our purpose at least](https://gitlab.gnome.org/GNOME/glib/-/blob/main/glib/gkeyfile.c#L1277).
I already pointed that out [here](https://github.com/geany/geany/pull/3413#issuecomment-1445242422) but as I noted its _not_ documented. Relying on undocumented features seems risky. @techees changes remove that reliance.
set lexer.props.allow.initial.spaces=0 in filetypes.conf, which we probably shouldn't…
Yes we should, to match the parser behaviour you note further on.
For history, the property originates from https://github.com/geany/geany/commit/00bc23cec4d86d5e18752dc38034978ac18f96..., which commit message confuses me, as I understand it as meaning the exact contrary to what it actually does…
Well the default value in Scintilla is 1 which matches the comment, but then Geany sets it 0 which is correct (IMNAAHO).
I'm not saying we shouldn't apply your changes here, they do look better to me. I'm just trying to be (too) thorough.
So this is ok and arguing about the lexer prop can occur on another issue/PR, then I agree and LGBI.
@techee thank you for doing a boring repetitive job
Yes we should, to match the parser behaviour you note further on.
One thing I forgot to mention is that it's the same commit mentioned above that also modified the parser not to allow leading whitespaces…
Well the default value in Scintilla is 1 which matches the comment, but then Geany sets it 0 which is correct (IMNAAHO).
You seem to have a lot against leading whitespaces here, far beyond whether we should or shouldn't use them in these specific files, why is that? My personal *feeling* is that we shouldn't *use* them in our conffiles, but that we should support them in the file type; I don't see any reason why to limit this, is there really valid cases where it causes problem to support it? @eht16 as the author of the mentioned commit, can you remember the reason ages later?
You seem to have a lot against leading whitespaces here, far beyond whether we should or shouldn't use them in these specific files, why is that?
Not a _lot_, but you are right, the point is beyond just Geany conf files.
There were, and possibly still are, a lot of things that used ini like conf files (or has everyone gone JSON/TOML?) and a lot of different parsers, not just Glib, so who knows if they accept leading spaces. Given that Geany does not know what parser is going to be used my feeling is that Geany should not _by default_ encourage something that might not be legal. So 0 is the right default for Scintilla, isn't the property settable in the filetype file if the user really wants to use indents?
Maybe the property should be in the filetype file set to 0 so users can find it to modify it (cycling neatly back to the original point ;-)
And of course 0 matches the parser (upstream is the same, having come from this one I guess).
Given that Geany does not know what parser is going to be used my feeling is that Geany should not _by default_ encourage something that might not be legal.
I'm rather thinking the contrary, that Geany should support wider use cases so long as it doesn break another one. And thus I would think the parser should accept leading whitespaces as well.
And I don't think that we can rely on this file type to guarantee any particular syntax, for example the parser accepts `;` comments, and I didn't look again but I guess the lexer has similar extension that breaks your expectations of the highlighting being a validator or similar.
for example the parser accepts ; comments, and I didn't look again but I guess the lexer has similar extension that breaks your expectations of the highlighting being a validator or similar.
Scintilla accepts `;` as well, after all its the original comment in MS `.ini` files, not `#` these Unix come lately guys use ;-P
But leading spaces are orthogonal to `;` / `#`, IIRC there are parsers that do what the parser here does, look at the first character of the line only, be it `#` or `;` (or `[`).
As I said I don't care a _lot_ so if an import of the parser from ctags changed its behaviour and the default Scintilla was made the same thats fine.
Anyway lets stop sidetracking @techee's excellent PR.
@b4n approved this pull request.
Looks good, as usual
Merged #3428 into master.
My personal feeling is that we shouldn't use them in our conffiles,
Agree. I think that apart from the reasons mentioned above, it just looks nicer in the conf file when all commented-out lines start with `#`.
but that we should support them in the file type; I don't see any reason why to limit this, is there really valid cases where it causes problem to support it?
I don't have a strong opinion on this. There are two possible views:
1. Display all possible conf files nicely syntax-highlighted. 2. Encourage users to use the safest conf file format possible by not highlighting a possibly invalid format (the specification here https://specifications.freedesktop.org/desktop-entry-spec/latest/ar01s03.htm... doesn't say anything about leading or trailing spaces so it may be implementation specific).
github-comments@lists.geany.org