@cousteaulecommandant pushed 1 commit.

315cb9434d7eef9732e48c938350c99e31542216 Modify (fix) test accordingly

Please submit the changes to upstream Universal Ctags, we need to be able to import it unchanged, we don't have the resources to keep patches that will have to be re-applied every time ctags is updated.

The file I modified is `ctags/parsers/geany_matlab.c`, which looks like a modified version by Geany (it has "geany" in the name!), is it not? I just checked in [the universal-ctags project](https://github.com/universal-ctags/ctags/blob/master/parsers/matlab.c) and their matlab.c is completely different to the one in Geany, so I guess Geany just uses its own modified version. I'm a bit confused about all this process (this is my first "real" commit to Geany with actual C code in it).

(There's still the small issue of all the tests failing; I'm working on it)

PS: maybe universal-ctags's `matlab.c` was removed for historical reasons and the devs forgot about it, and it should just be added back replacing Geany's own modified version? I can test if the upstream version works too.

Ahh, ok, time for history lesson, stone age ctags became exuberant ctags which went unmaintained for a long time during the dark ages, before the renaissance when universal ctags was forked to provide what it calls "a maintained ctags implementation" which has now become the modern ctags implementation.

During the dark ages when changes were not being accepted in ectags, changes were made to Geanys copy. Since the renaissance some of those changes have been pushed upstream if acceptable and Geany and uctags have been unified, some have simply been replaced by upstreams which are significantly better (C/C++), and some are so different that they are too hard to integrate into changed upstreams so the modified Geany version has been kept.

I guess Matlab is in that category.

Also the upstream matlab parser is a regex parser which Geany historically did not accept (@techee is that still the case?) so upstream couldn't be merged, so the olde version is kept.

Pending what @techee advises I guess the options are to replace Geany's one with upstream if regex parsers are now supported (to my quick look upstream parses classes that Geany's doesn't, so its "better") or to keep maintaining Geany's unique version until regex parsers are supported.

@cousteaulecommandant pushed 1 commit.

8d1c3af177c2dfb382aa4452f2c56d9198cfd731 Modify (fix) test accordingly

Maybe I should add a few extra lines to that test with some of the new corner cases I'm now capturing, like the case with = in a comment.

Certainly more is always better, in case someone else breaks it at some point :-)

As per the level of support for regular expressions, I see no other ctags parser using tagRegexTable, so I'm not sure it's supported. (But even if it is, I'm not sure it's a good idea to use it only for Matlab.)

Yeah, as per the comment in #3170 there are no others, and there is a common apprehension that regex parsers are slow, so having them run between keystrokes is an issue.

I notice that other tests have a much more complex .tags file which seems to include an argument list,

You are running up against the limits of my knowledge of ctags/tagmangler (the expert referred to previously is _NOT_ me) but IIUC the arglist is a separate field on the tag, its not part of the tag name (for the reasons you made this PR). Not sure how that is added, maybe look at a parser that does it?

@cousteaulecommandant pushed 1 commit.

78870da6dc37562e67ae8ee43da43ca9b210ba46 Improve ctags tests for Matlab

Regarding the regex ctags version of the parser vs Geany's version of the parser: we could use the regex ctags version, I was just thinking that since we have the hand-written version in Geany already, it might be a base for a hand-written parser that could eventually be submitted upstream so I kept the `geany_` parser. Hand-written parsers tend to offer better flexibility in parsing and are much faster than regex parsers. But before such a parser could be submitted upstream, it would have to offer all the functionality the current regex parser offers.

I think the custom parser is currently a bit messy and could use some restructuring, but your point is valid. Right now, I think the only real advantage of the regex version of the parser is its readability, but it doesn't seem to be really leveraging the full power of regexes -- it is rather simple and can probably be translated to "plain C" easily. (Also, at first glance, it seems that those regexes aren't too good either; for example, I believe the second one will match `functionality = 42` too.)

Re: speed, I see four "levels" in which the parser can be implemented: 1. Compare characters one by one 2. `strncmp()` and `strstr()` 3. **`sscanf()`** :bulb: 4. Regular expressions

I think the regex parser could be easily re-implemented using sscanf() as a faster alternative to regex, so if that's an option I think it'd be an elegant solution -- readable, efficient, and less prone to errors than options 1 and 2.

Something like ```c if (sscanf(line, " function [%*[^]%]] = %[A-Za-z0-9_]", buffer) == 1) ... if (sscanf(line, " function%*[ \t]%*[A-Za-z0-9_] = %[A-Za-z0-9_]", buffer) == 1) ... ``` etc. (where `" "` matches zero or more whitespace chars, `"%*[ \t]"` matches one or more spaces/tabs, `"%*[^]%]"` matches anything but `]` and `%`, etc -- it's not incredibly readable, but it's fast.)

So, what do you think? Would `sscanf()` be fast enough, or better to keep matching individual chars and substrings?

Probably could be done by checking if after

p=(const unsigned char*) strstr ((const char*) line, "struct");

`p-1` and `p+6` are not alnum (plus all the necessary range checks).

That still won't ignore words in strings (and maybe other corner cases). Honestly I think I'd entirely ditch parsing structs; knowing that a certain variable at a certain point in the program is a struct isn't really that relevant, and universal-ctags doesn't do it anyway. Class parsing would be more useful. For a similar reason, I'd avoid parsing all variables as universal-ctags does; having a list with EVERY variable assignment in EVERY function in the script seems excessive. (However, it might be a good idea to list `global` and `persistent` variables.)

Perhaps the sscanf method can find strings and comments first and not look for other stuff inside them?

The regices(?) defined in upstream ctags are meant to match from the beginning of the line (notice the `^` at the beginning), so they're not going to accidentally match things inside an end-of-line comment or a string.

If you want to ignore definitions inside _block comments_ (and I considered modifying my current PR or creating a new one for that) then the good news is that that's relatively easy to do, since a block comment is always going to be delimited by lines containing **only** `%{` and `%}` (possibly with some whitespace) and nothing else, so there's no risk to accidentally interpret the content of a string or comment as a block comment delimiter. The only difficulty in this regard is that block comments can be nested (think of C's `#if 0 ... #endif`), but that's easy to solve with a counter.

As for end-of-line comments, those always start with `%` or `...` (excluding those in a string, but you won't find strings in function definitions), so they are easy to avoid. And strings don't span multiple lines. So I think that other than the rare `%{`...`%}` case, it should be straightforward.

Don't know how fast sscanf is, its possible it won't be any faster than regexes since its still **scanning the input more than once.** Thats what is slow for regex parsers, the fact that multiple regexes are applied to the same input, not the fact that a well optimised regex library like PCRE is slow. Repeat scans is one thing well written character by character parsers try not to do.

My understanding is that the scanf family functions only need to parse one character at a time, and **never backtrack or look ahead** more than one character, so they don't need to perform multiple passes on the input. For example, if you do `scanf("%d", &i)`, that's going to read character by character from stdin, and as soon as it finds a character that doesn't match an integer (it's not a digit or a leading whitespace/sign), it'll put that one character back into stdin (see `ungetc()`) and return. Think of it as a [possessive regex](https://www.regular-expressions.info/possessive.html). Regular expressions, on the other hand, have to try to match different combinations until one of them works (unless they're [possessive](https://www.regular-expressions.info/possessive.html)), hence the need for backtracking and multiple passes and inefficiencies.

For example, when matching the string `abcde123` against the regex `([a-z]+)([aeiou]+)` it will succeed, because the first capture group will match `abcd` (even if it could match all of `abcde`) and the second part will match `e`. But the scanf pattern `%[a-z]%[aeiou]` will fail, because the first specifier will just eat all the letters and not leave any for the second.

So in other words, `sscanf()` is a glorified char-by-char parser that only needs to look ahead one character at most. And it's also probably very well optimized, so it might be even faster than writing the parsing state machine yourself. Maybe we could just write some tests and time them.

The writers of the Python parser took the "every assignment is a declaration" approach and the Julia parser writers took the approach "no assignment is a declaration". So Python is a precedent for having all the names available, and I haven't seen too many complaints about it.

I had never noticed this, and it feels kinda wrong that the same variable can be "declared" in multiple places, but then again, Python programs are usually a bunch of functions/classes with maybe a few "file-scope" variables, and the parser ignores assignments performed inside a function (which are local to the function). So for a typical Python file structure, it makes sense to assume that every assignment done outside of a function is some sort of "global" variable. But one could also make a Python script where most/all the code is outside of a function and is executed directly (this would look a bit ugly in Geany because of all the variables, but that's the price to pay if we want a "normal" module-like Python file to look good).

Similarly, Matlab files can be of two types: either "scripts" where all the code inside is executed or "functions" containing one or more function definitions. Maybe we can just disable variable assignment detection when inside a function (i.e., when a line defining a function has been scanned before), and then we'd have Python's behavior.

However, I'd argue that in the case of Matlab files, there's nothing similar to C's "file-scope variables" since you'll never mix function definitions and variable declarations, so maybe it's a bit pointless to parse variables. But I'm OK with it as long as the ones in functions are excluded.

...

Honestly I think I'd entirely ditch parsing structs;

Don't know matlab enough to comment, but if no Matlabbers object I guess its ok if upstream doesn't do it.

I have some experience with Matlab and I'd say structs aren't used that often, or at least I don't use them often (and they're far from the only type of data structure). Plus using `struct` explicitly isn't the only way to declare a struct (and I'd say it's rarely done); just doing `a.b.c.d = 42` will declare `a` as a struct containing a struct containing a struct if `a` doesn't exist yet. I think in practice I'd only ever use `struct` explicitly if I wanted to "reset" a struct variable to the empty struct.

Regardless of what direction does Geany go in a future regarding Matlab parsing (upstream ctags, regex, sscanf, strncmp...), can this PR be merged? As far as I understand it is ready to merge, and solves an issue with Matlab file parsing.

I still have some interest in improving this in a future (`%{...%}` comments, classes, etc), but for the time being this PR solves the issue at hand.

@cousteaulecommandant pushed 3 commits.

56afcc71d19fb6ed26441b6eaf5adddc620999d1 ctags: Matlab: function name only, without args 568806ea78e98ac2e82b288fa15d443600631294 Modify (fix) test accordingly 19feb68c28427d423015b71428962ed766fc8c57 Improve ctags tests for Matlab

I had a look at the code and it appears to do the right thing. The changes seem simple enough that they shouldn't cause any problems - so even though we are close to release, I'm merging this.

Thanks!