@techee pushed 1 commit.

0633a28 Remove -Werror=aggregate-return from travis flags

@techee pushed 1 commit.

0dd9f3a Eliminate use of regex from read.c

Alright, so it finally builds and it also builds and runs on macOS, I've just tried.

And beside the process, what have uctags done that suddenly needs +11800 lines of code?

@codebrainz @techee but what does all that code actually DO? Why has it accumulated over 10 years (if thats the reason)? What benefit does it bring to include it in Geany?

@codebrainz @techee but what does all that code actually DO? Why has it accumulated over 10 years (if thats the reason)? What benefit does it bring to include it in Geany?

In fact I think it has accumulated over something like 15 years - it was first part of the tag manager (which is from 2001), then it was taken over and modified by Anjuta and then by Geany. Each project was adding its own stuff on top and the differences started getting bigger. I think the original fork also just deleted lots of lines from ctags which weren't used by TM. I could do the same now too but it makes the code harder to compare against uctags which is why I kept it and why there are 10000 more LOCs.

But there are also features we want - it's mostly things which can be used by parsers like a simpler mechanism to create scope information (called cork) and the possibility to run a subparser from a parser (I used this in the upstream HTML parser to use real parsers for embedded javascript and CSS).

During this year I started adding things we need in Geany into upstream ctags (like TM and other things) so there's not much missing there we would need in Geany. The ultimate goal is there's no Geany fork at all and that we just treat ctags similarly to Scintilla - development just upstream, taking over all the code once a while.

@techee thanks for the explanation, can you explain more what is "cork" and what it provides? This is more for understanding than directly relevant to the PR.

You pointed out the concern, is there anything that is not needed in Geany, thats in that code because ctags needs it (like for instance the stuff that outputs ctags files). I know Geany abandoned the "small and lightweight" some time ago, but this is adding two extra `editor.c`s worth of code to the executable.

I don't suppose you could get uctags to provide a `libctags` like Geanys `libmain`?

Binary-size wise libgeany.so size looks this way:

17942824 before 18533832 after ```

I guess that's with default `-O2` and `-g` without stripping symbols?

Here's what I get with different options (always using `make install-strip`):

``` Master branch w/ '-O4': 2.8 MB This branch w/ '-O4': 3.0 MB This branch w/ '-O4 -flto': 2.8 MB This branch w/ '-O4 -flto -fwhole-program': 2.9 MB This branch w/ '-Os -flto': 2.6 MB ```

The "cork" thing is something that helps constructing scope information by referencing the parent tag. You write something like:

parent_tag = makeTag(...); chlid_tag = makeTag(parent_tag, ...);

and arbitrary nesting of these and it automatically generates scope for you (you also have to statically specify scope separator before). Don't ask me why it's called cork though ;-).

So thinking about it, that means it still only handles named scopes (ie those that have parent tags), still not lexical scopes ie { to }? If so it still seems a lot of code.

The great majority of the added code is nothing useful for Geany (processing command-line parameters of ctags, support of different output file formats, etc.). The only useful thing which this patch brings is the mentioned cork thing and the ability to run nested parsers from within another parser. Adding this would be less than 1000 LOCs (rough estimate).

That was what I suspected.

1. Syncing parsers.
2. If possible getting rid of Geany-specific code we still have.
3. Bringing the library support to ctags.
4. Refactoring ctags so only the necessary things compile for a library.

I suggest doing it in reverse order, wait until ctags is made into a library then do one change when it has a stable API. Bringing in all the unused crap and then removing it and interfacing Geany to whatever the ctags API is gonna be is wasted work IMO.

I suggest doing it in reverse order, wait until ctags is made into a library then do one change when it has a stable API. Bringing in all the unused crap and then removing it and interfacing Geany to whatever the ctags API is gonna be is wasted work IMO.

One of the important parts of this pull request was to learn what we actually need in the API which wasn't clear before and what the differences between Geany and uctags are (which wasn't clear at all before). I wouldn't know how to create the API without this.

There's nobody working on libraryfication of uctags upstream. Waiting for it to happen would be pointless - it won't happen by itself. The only project that uses uctags as a "library" is us and the and it's us (probably me) who will have to do it. Doing it properly will take some time - I'd most probably start doing it "improperly", i.e. bringing the API to uctags the way it's done in this pull request and then starting to refactor it to turn it into a proper library. This will take time however and having this pull request in will make it possible to start working on syncing parsers. In addition the state after this pull request is better than before - it's MUCH easier to bring changes to Geany from uctags than before. So even if it turns out I don't have time to work on uctags or it turns out the uctags project doesn't want the library change (or whatever else), it's better to have this in.

That said, if this PR includes any files that are not used in any way, it makes no sense to include them, we should do like we do with Scintilla and leave out completely unused files, IMO.

I just did it for a few files (should be around -4000LOCs in total) but even though some more files look they might be removed too as we don't need their implementation, it's not so easy as they are called from other files (in functions which don't get called by Geany). But this is what would need quite some refactoring and it's something I'd leave for the uctags patches.

• GRegex vs GNU regex - I kept GRegex in lregex.c (but synced all the rest) but we should think whether not to convert back to GNU regex

Well… I'd rather not bundle yet another library, especially as we have the same feature already available in a dependency.

• isIgnoreToken() works differently than in uctags.

Does it? Isn't just the API different?

If it simply just dropped certain tags, we could move the logic into TM but right now, it helps the C parser with parsing by suggesting whether the contents of braces should be dropped too. I'm not sure if we need to preserve this functionality (never used the ignored.tags file myself) but if we move to the new cxx parser we'd have to keep this diff unless there's some interest for this in uctags.

Well, the point of ignore tokens is to mitigate the confusion macros can cause on the C parser. For example, I have a custom ignore.tags with those: ``` G_GNUC_DEPRECATED_FOR+ G_GNUC_PRINTF+ G_GNUC_SCANF+ G_GNUC_FORMAT+ G_GNUC_* ... __STL_NOTHROW __THROW GIT_EXTERN=extern cairo_public GEANY_DEPRECATED GEANY_PRIVATE_FIELD GEANY_DEPRECATED_FOR+ ``` so various source files parse a lot better, because the parser just performs as if those weren't part of the source. See it as the poor man's C preprocessor.

...

isIgnoreToken() works differently than in uctags.

Does it? Isn't just the API different?

One difference is in how isIgnoreToken() is used in c.c. In uctags parsers (or c.c in particular) don't call this function at all - this function is just called in main and based on it some tags are filtered-out. However, in Geany's c.c this function helps to the parser to determine what to do - if the ignored token is e.g. a macro with parameters, everything is skipped until ")".

This also translates to the ignore file format - in the Geany version there's the "+" which indicates that parentheses should be skipped and "=" which specifies with what the token should be replaced. There's nothing like that in uctags.

Personally I'd be for removing this functionality as if we want to preserve it, we'd have to add it to the new cpp uctags parser which doesn't use this. As I said, I don't use the ignored file myself so I'm not the right guy to ask whether it's something important for others.

techee commented on this pull request.

while (1) { nl = nestingLevelsGetCurrent(nestingLevels); - if (nl && nl->type >= kind) + e = getEntryOfNestingLevel (nl); + if ((nl && (e == NULL)) || (e && (e->kind - AsciidocKinds) >= kind))

Because it can be set to CORK_NIL in makeAsciidocTag() when vStringLength (name) == 0.

techee commented on this pull request.

{

- e.extensionFields.scopeKind = &(AsciidocKinds [nl->type]); - e.extensionFields.scopeName = vStringValue (nl->name); + e.extensionFields.scopeKind = &(AsciidocKinds [parent->kind - AsciidocKinds]); + e.extensionFields.scopeName = parent->name;

Yeah, again copied from rst.c, probably unnecessary.

techee commented on this pull request.

contextual_fake_count = 0;

Assert (passCount < 3); - cppInit ((bool) (passCount > 1), isInputLanguage (Lang_csharp), isInputLanguage (Lang_cpp), &(CKinds [CK_DEFINE])); + + kind_for_define = CKinds+CK_DEFINE; + + cppInit ((bool) (passCount > 1), isInputLanguage (Lang_csharp), isInputLanguage(Lang_cpp), + kind_for_define);

Yeah, not necessary for our c.c. It was taken from uctags where there was a branch where the value depended on whether parsing Vera or c.

techee commented on this pull request.

• {

- GStatBuf s; - - /* load file to memory and parse it from memory unless the file is too big */ - if (g_stat(file_name, &s) != 0 || s.st_size > 10*1024*1024) - parse_file = TRUE; - else - { - if (!g_file_get_contents(file_name, (gchar**)&text_buf, (gsize*)&buf_size, NULL)) - { - g_warning("Unable to open %s", file_name); - return FALSE; - } - free_buf = TRUE; - } - }

This is now done in ctags - see getMio(). The mem. size is just 1MB there instead of 10MB here but for normal sources it doesn't matter (and since ctags uses fgets() for input reading, even the file-based access is quite fast).

techee commented on this pull request.

return NULL;

+ + start = strdup (vStringValue (signature));

OK

static bool createTagsWithFallback1 (const langType language, ...

Doesn't this miss initializeParser (language);?

No. We call

``` /* make sure all parsers are initialized */ initializeParser (LANG_AUTO); ```

inside ctagsInit() in ctags-api.c for all languages only once and don't need to do it every time when parsing.

static void attachEndFieldMaybe (int macroCorkIndex) ... Would be nice to have that :)

Do you plan to use cork in c.c? I can add it (or it can be added later when cork in c.c is used).

I have been thinking how to proceed when we decide to switch to the uctags new cxx parser. Since we'll still need c.c for other languages and since the lcpp.c file differs significantly between Geany and uctags, it might be necessary to have lcpp-old.c (current Geany version) which would be used by c.c and lcpp.c (uctags version) which would be used by cxx. Then C/C++ could be removed from c.c and you suggested separating Vala to a new parser and then it might be easier to either merge both lcpp implementations.

@techee pushed 9 commits.

8455f8e lcpp, c: Fix signature collection d5823ce c.c: Reinitialize rescan variable in case longjmp() happens 634719a c.c: Eliminate useless kind_for_define variable cf6653a Don't check if kind is enabled for library 6176846 Remove unnecessary use of AH_TEMPLATE 3e8365c asciidoc, rest: Remove unnecessary scope assignments e5b5326 python: Rename corkPair struct to corkInfo df0ae9b python: minor cleanups bf4eb8f lcpp: use eStrdup() instead of strdup()

Great! Yeah, pure fear of the size of the patch made me not to touch it for such a long time (and it wasn't that bad at the end, thought it would take me a week to remember what it does).

By the way, has anything happened with travis? Seems no checks are running.

@eht16 there have been no results displayed on the PRs since beginning of January. I checked other projects and they are all showing results fine, its just Geany. What changed on new years eve? ;)

@eht16 its already got the checks in progress message, thats better :)

Woohoo, working again. Sorry for the trouble.

Sort of. In Travis (when logged in as the "geanyadmin" user), builds can be restarted. But we need to do this per build. If you want me to do, I could try to restart the last build of the recent PRs back to beginning of January.

@techee OK, I hope I'll have some time to try and tackle that one again. However, do you think I should review this as is, or first try and update it with more recent U-CTags? Basically it depends on how easy you think it'll be to sync with uctags master (hopefully easy enough as it's one of the goals :smile:), but if there's tricky part maybe it could be worth updating the year+ changes I caused and then tackle that monster.

@techee pushed 4 commits.

9b44e9f Fix parsing with sub-parsers 25e8ebc Fix indent 248c970 Add missing void 99e0f20 Merge branch 'master' into ctags_sync_main

techee commented on this pull request.

+#include <stdio.h> +#include <string.h> +#include <errno.h> + +static bool nofatalErrorPrinter (const errorSelection selection, + const char *const format, + va_list ap, void *data CTAGS_ATTR_UNUSED) +{ + fprintf (stderr, "%s: ", (selection & WARNING) ? "Warning: " : "Error"); + vfprintf (stderr, format, ap); + if (selection & PERROR) +#ifdef HAVE_STRERROR + fprintf (stderr, " : %s", strerror (errno)); +#else + perror (" ");

Yeah, we'd need some proper API for that which we don't have. The problem I was running into was that in tag definitions of individual parsers some tags were disabled by default and "make check" failed because of that. So it was either disabling the enabled check here or changing all the affected parsers tag definitions and this was just easier.

If we continue with this speed, we might have it merged around the time we are retired ;-)

Yep, we have a chance of finishing it in this lifetime! :)

1. The lcpp problem

https://github.com/geany/geany/commit/6f692112e39fa0714d882332b8f935419fd69a...

seemed to be fixed upstream so I kept the version from this pull request and discarded your patch.

Yeah, apparently it was fixed separately: https://github.com/universal-ctags/ctags/commit/4ceab02734d91bcbaf765be0f4e4...

OK for the other two. I'll check the rest of the changes in the coming days (yes yes I will -- or at least try hard)

@elextr I don't think so. We're almost there, and updating parsers basically relies on a lot of this PR (it's the point of it) because UCTags added quite a few facilities and changed things and parsers are using those.

And regarding the status of this, yes it took a long time, but I'm pretty sure we're almost there (last review was close to merge state), I'm committed to this and work a little on it, although these past few weeks I focused on what would make 1.34. It's one of the PRs I'd like to merge early in 1.35 if we don't encounter no new unexpected issue.

@b4n Well, I didn't get a sense that you were making progress, will hassle you immediately after 1.34 release so it can be committed ASAP to allow for some time to mature before 1.35.

[PS am only accepting it so I can have the new C++ added after the infrastructure is fixed :grin: ]

b4n requested changes on this pull request.

OK, here's my latest with the help of GCC. See also https://github.com/techee/geany/pull/1 which contains some small stuff that I think are relevant, but please review them, don't take them as gospel.

Apart from that, I think the next thing is merging an see how it goes, as as far as I can tell it works very well.

pushNarrowedInputStream (language,

startLine, startCharOffset, endLine, endCharOffset, sourceLineOffset); #ifndef CTAGS_LIB tagFileResized = createTagsWithFallback1 (language); +#else + /* Simple parsing without rescans - not used by any sub-parsers anyway */

I don't really care right now because as you pointed out there's no use case yet, but you mentioning in the commit message that "the C and Fortran parsers" being the only parsers using retry mode and that they do not support sub-parsers makes the retry case irrelevant is not correct: nothing prevents a parser from calling the C or Fortran parsers as sub-parsers. IIUC, the upstream Flex or so parser actually does this.

But as said, don't worry too much about that right now; we can fix this stuff when we actually have use for it.

else if (c == SINGLE_QUOTE)

break; else if (c == NEWLINE) { - ungetcToInputFile (c); - break; - } - else if (count == 1 && strchr ("DHOB", toupper (c)) != NULL) - veraBase = c; - else if (veraBase != '\0' && ! isalnum (c)) - {

I can't find these lines being suppressed upstream, so I assume it's a local change. However, you should then remove the variable declaration for `veraBase` as well.

vString *const parent, int is_class_parent, const char *arglist)

{ tagEntryInfo tag; + int corkIndex; + int fqCorkIndex = CORK_NIL; + const struct corkInfo nilInfo = {CORK_NIL};

unused variables `fqCorkIndex` and `nilInfo`, any reason to have them?

vString *const parent, int is_class_parent, const char *arglist)

{ tagEntryInfo tag; + int corkIndex; + int fqCorkIndex = CORK_NIL; + const struct corkInfo nilInfo = {CORK_NIL};

I see they were upstream in the old Python parser, but they were used at the time.

if (vStringLength (name) > 0) { + tagEntryInfo *parent = getEntryOfNestingLevel (nl);

This variable is unused, but upstream (*rst.c*) uses it to mess with the scope (which seems stupid, but well). Unused variables have that issue that people have a tendency to remove them… which generally seems legitimate. BTW, if we remove this, then `nl` gets unused, but we still gotta call `getNestingLevel()` because it does the `nestingLevelsPop()` stuff needed. What do you think?

if (vStringLength (name) > 0) { + tagEntryInfo *parent = getEntryOfNestingLevel (nl);

same here than in *rest.c*, `parent` is unused, same question applies.

techee commented on this pull request.

pushNarrowedInputStream (language,

startLine, startCharOffset, endLine, endCharOffset, sourceLineOffset); #ifndef CTAGS_LIB tagFileResized = createTagsWithFallback1 (language); +#else + /* Simple parsing without rescans - not used by any sub-parsers anyway */

OK, good point. I wanted to avoid re-parsing within subparsers because this would complicate things in the tag manager - if re-parsing happens in the main parser, we clear the tag list and start over. However, if such a thing happened within a subparser, we'd have to have some way to clear just the part of the tags generated by the subparser while leaving the tags generated by the main parser and we currently don't have anything like that in place.

Personally I'd just sacrifice re-parsing in this case (i.e. when using flex running cpp parser as the subparser) to simplify things. If I remember correctly, re-parsing happens quite rarely, at least when I tried the linux kernel sources.

@b4n Thanks for the patches, I've added them to this pull request and also removed the currently unused code from asciidoc and rest parsers.

I guess it's kind of safe to assume now that there won't be too many changes in this pull request, right? If so (and if I have some time left this week), I can start checking what changes have been made in the current ctags master in the last two years which we might need to have all the necessary stuff from main so we can just take over parsers from ctags.

Just for info, I've synced ctags to the state where the majority of parsers can just be copied from upstream ctags without any modification. The code is here

https://github.com/techee/geany/commits/ctags_sync3

and I'll convert it to a proper pull request once this pull request is merged. As part of this branch I used the upstream versions of html/jscript/css parsers to be able to debug subparser support and it seems to be working pretty well now after some patches.

b4n commented on this pull request.

if (vStringLength (name) > 0) { + tagEntryInfo *parent = getEntryOfNestingLevel (nl);

It's incorrect, because I stated in my commend above, we still have to call `getNestingLevel()` because it's that function that calls `nestingLevelsPop()` when appropriate. Just removing the call removes this side effect, resulting in never removing nest levels.

This is pretty much exactly what I tried to warn about in the comment above when sating that *"have a tendency to remove [unused variables]"*, suggesting it's not so simple here. What we can do is call the function without storing its value, and possible rename it to be clearer.

techee commented on this pull request.

if (vStringLength (name) > 0) { + tagEntryInfo *parent = getEntryOfNestingLevel (nl);

Done.

b4n approved this pull request.

Merged #1263 into master.

So… more than 2 years after, this is merged! Thanks, and we're ready for more :)

Hi everyone !

If I may, and in my humble opinion, it would be a mistake to drop support for COBOL :

- There's a huge heritage of code lines (220 out of 310 billion code lines developed worldwide, estimate at the beginning of this year) - There are still many companies on the mainframe (including banks and insurance companies) - Geany is one of the few to take it into account and it is an argument - Companies running on AIX mainframe solutions (Linux) need a local IDE - On my last three missions, I managed to replace Notepad+++ by Geany (integration of the compilation exe on F5), especially with the filedef I provide you today

So yes I know that COBOL isn't modern, sexy but it is widely used. And in this case, why not also abandon Assembler, Fortran or Pascal ? (and I spare you the REXX filedef I developed) ;)

@DansLeRuSH its great that you have contributed filetype improvements.

You're welcome, I love Geany and if I could help :blush:

But if Cobol is so useful then Cobol users should step up and provide support for the parser.

You're absolutely right ! I told so to my past and actual colleagues. Perhaps improving actual possibilities could help :thinking:

Geany is a totally volunteer project, its is not supported by any companies. Those banks and companies you mention should be able to have their people contribute if they want support to continue

The main problem here is that those companies love to give a lot of money to historical software suppliers, have no trust in opensource projects and there's a gap between CIO/supplier trade agreements and developpers needs. But yes I'm agree with you.

(sadly I suspect they all use Eclipse).

You're partly right if it's a classical mainframe on z/OS. The problem here is that IBM has built a solution on top of Eclipse (IDz) but it's not open source and really expensive, small companies can't afford that.

The suggestion to abandon Cobol is because something needs to be done to continue support, and in a volunteer project, if no one contributes that, then support can not continue.

I'm aware of that and abover all, thank you for the hard work !

I'm not an expert on whats needed, but IIUC the problem is that the Cobol parser uses a different Regex library from the rest of Geany, and, as I asked above I'm not sure why thats an issue, and what is needed to make it work. Perhaps open a specific issue to discuss it if its an issue [pun intended].

I'm a proud daddy and I haven't so much time but I could help sometimes If you need :vulcan_salute: :nerd_face:

The filetype is still available.

Which means the changes in #2021 to the Cobol filetype are still useful since they apply to the highlighting and other non-parser functionality.