@b4n requested changes on this pull request.
Looks mostly good (see inline comments), but I'd really like a CTags test case
(you can import the one from uctags)
> @@ -0,0 +1,37 @@
+# For complete documentation of this file, please see Geany's main
documentation
+[styling]
+# Edit these in the colorscheme .conf file instead
+default=default
+commentline=comment
+block=function
+date=keyword
+number=number
+
+[lexer_properties]
+#fold=0
+fold.comment=0
why disable this by default? AFAICT we usually have this enabled for other
languages
> +default=default
+commentline=comment
+block=function
+date=keyword
+number=number
+
+[lexer_properties]
+#fold=0
+fold.comment=0
+
+[settings]
+# default extension used when saving files
+extension=snx
+
+# MIME type
+mime_type=text/sinex
Doesn't seem to be an actual MIME type, but given it's pretty logical and not
reserved for something else it's probably fine
> +# using word matching options
+#wordchars=_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789
+
+# single comments, like # in this file
+comment_single=*
+# multiline comments
+#comment_open=
+#comment_close=
+
+# context action command (please see Geany's main documentation for details)
+context_action_cmd=
+
+[indentation]
+#width=4
+# 0 is spaces, 1 is tabs, 2 is tab & spaces
+type=0
is space indentation *mandatory* for this filetype? If tabs are OK as well, it
shouldn't be hard-coded here to someone's preferences, that should be up to the
user to choose (possibly adjusting it for this filetype)
On scintilla/scintilla_changes.patch:
kudos for updating the patch! :+1:
> + blockNameCopy(blockNameStart, (const char *)line);
+ initTagEntry (&e, (const char * const)blockNameStart,
K_BLOCK);
+ inBlock = true ;
It would be better to check for non-empty block names, as it is if one writes
```sinex
+
-
```
one gets:
```
(geany:1234): Tagmanager-WARNING **: 01:23:45.678: ignoring null tag in
/path/to/file.snx(line: 1, language: SINEX)
```
It's not a showstopper as it won't crash and CTags accepted this, but it'd be
better to iron this out. Maybe
```suggestion
blockNameCopy(blockNameStart, (const char *)line);
if (*blockNameStart)
{
initTagEntry (&e, (const char *
const)blockNameStart, K_BLOCK);
inBlock = true ;
}
```
On meson.build:
This doesn't build: you need to reference the CTags parser as well (currently
we get a missing symbol when linking):
```diff
diff --git a/meson.build b/meson.build
index ee1022544..b6672aec6 100644
--- a/meson.build
+++ b/meson.build
@@ -716,6 +716,7 @@ ctags = static_library('ctags',
'ctags/parsers/rust.c',
'ctags/parsers/sh.c',
'ctags/parsers/sh.h',
+ 'ctags/parsers/sinex.c',
'ctags/parsers/sql.c',
'ctags/parsers/tcl.c',
'ctags/parsers/tcl.h',
```
> + styler.ColourTo(endPos, SCE_SINEX_DEFAULT);
+ }
+}
+
+
+// Colourization logic for a whole area
+// The area is split into lines which are separately processed
+void ColouriseSinexDoc(Sci_PositionU startPos, Sci_Position length, int,
WordList *[], Accessor &styler) {
+ // initStyle not needed as each line is independent
+ std::string lineBuffer;
+ styler.StartAt(startPos);
+ styler.StartSegment(startPos);
+ Sci_PositionU startLine = startPos;
+
+ for (Sci_PositionU i = startPos; i < startPos + length; i++) {
+ lineBuffer.push_back(styler[i]);
nitpick (ignore, unless you'd be willing to spend the time upstream): why use a
copy of the line, rather than the usual state machine Scintilla lexers use?
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/4449#pullrequestreview-3601199501
You are receiving this because you are subscribed to this thread.
Message ID: <geany/geany/pull/4449/review/[email protected]>
