IMHO this should be rejected as a single PR and made as a series of individual PRs commits that can be reasonably reviewed and committed individually. Like this its unreviewable IMO and will take ages to get committed.

I'll take a better look tomorrow, but if each commit is reviewable it should be fine.

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160#issuecomment-237091900

@techee pushed 1 commit.

9c4c113 Remove some additional functions from ctags which we don't need in Geany

--- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/1160/files/ea72ecc00fc22f24eced3ffce7c7d...

@techee pushed 2 commits.

7e0845b Make PathDelimiters match the declaration in header eb45c80 Always define ExecutableName variable

--- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/1160/files/9c4c113488f6d5743c79865c74f2d...

There is no technical point, but there is a social point. The point would be to allow the changes to be easier for committers to review in small digestable chunks, without having to split the changeset themselves, they are doing it in their own time, don't try to take up their free time in a big chunk. The whitespace commits are exactly the sort of thing that can be done in independent chunks. If its a small chunk with only whitespace changes I can look at it and help @b4n, but if its a big lump I'm not going to take that much time out of the other things I'm doing.

There's no reason to spend much time on reviewing the whitespace stuff - all the files will eventually get replaced by the current uctags files. I think it would be possible to simply grab all uctags files and put them into Geany's ctags directory (and adjust to our needs) but since there have been something like 12 years of independent developments of both ctags versions I want to make sure I don't miss some of the Geany's patches of ctags so I'm doing it gradually.

Thats fine, but that should be separate from whitespace changes.

It is a separate commit but I couldn't make it a separate pull request at this time - it depends on the whitespace stuff and if I made two separate pull requests affecting the same lines it would lead to terrible merge conflicts (which I really don't want to resolve). But if it's the preferred way I can drop this patch for now and once/if the rest gets merged, open a new pull request for it.

By the way, the majority of the code the patches touch is a dead code in Geany - it's things like reading source files, creating ctags files, reading command-line parameters etc. which we don't use at all in Geany. So really no need to spend much time here.

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160#issuecomment-237414504

I forgot to ask, why do we have dead code?

And why not do the whitespace changes file by file as you merge the ctags and Geany versions?

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160#issuecomment-237417540

I forgot to ask, why do we have dead code?

Because ctags isn't made to be a library - it's just a binary and we misuse it and use it in a library-like way. The result is that we use only some functions from some files while the rest is uninteresting for us.

In the future if I have time for this I'd like to add some real library support to uctags and there could be some ifdefs removing the dead code. But determining what is a dead code and what isn't is quite a lot of work and I don't want to do it in parallel with reducing the amount of changes between uctags and Geany.

And why not do the whitespace changes file by file as you merge the ctags and Geany versions?

Because you cannot do the merge file by file but rather something like diff group by diff group spanning multiple files. If file A uses stuff from file B and if I merge only file A independently, the result won't compile because of the missing change in B. And the dependencies are much worse than just A->B, it's more like A->B, A->C, A->D, B->A, D->A.

But the main purpose of this patch set is to get parsers exactly to this state where they can be merged file by file so everyone can grab his favorite language and merge it independently of the rest and shouldn't be affected by any additional changes in the "main" part of ctags.

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160#issuecomment-237476778

We could have a patch file similar to Scintilla which introduces the changes we need in ctags

Totally, and I support the end target of your changes, just not the process of getting there :smile:

I accidentally selected the "files changed" tab and it locked the browser for a time and used 1GB of memory +5328 line -5619 lines. IMNSHO its not a responsible thing to propose or to accept such a change. I greatly admire the quality of your submissions, but even you can't be sure whats in such a change, and poor @b4n really can't be expected to review it. There has got to be a better way, what about doing the whitespace first then? That will have no code changes and would be more reasonably reviewable and could even be a few files at a time.

my guts feeling is our c.c is better than the uctags one

Thats extremely disappointing since I think so much of ours that I turn off all uses of it but the symbols pane :smile:

(and the new uctags C/C++ one didn't seem to be much better when I tested, but presumably it will improve)

but @elextr, it sounds like a challenge for you :-)

I gave up self flagellation decades ago :smile:

And given your gut feeling above, no point anyway :frowning:

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160#issuecomment-237504880

...poor @b4n really can't be expected to review it. There has got to be a better way, what about doing the whitespace first then? That will have no code changes and would be more reasonably reviewable and could even be a few files at a time.

Reviewing it through "Files changed" would be crazy and would indeed be unreviewable. But it can be easily reviewed patch by patch - if you simply click the whitespace patch above, you get

https://github.com/geany/geany/pull/1160/commits/6bc579798716dbe94a171a016f9...

which is quite obvious (well, the github diff implementation screws up from time to time and doesn't show the same lines next to each other but there are not so many of those). Reviewing the patches individually is the same as if I created several pull requests (which isn't easily doable because of the patch dependency as I explained).

So I hope our poor old @b4n won't suffer much :-).

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160#issuecomment-237513070

@techee changing `g_` to plain POSIX has the following problems I'm aware of:

1. Changing anything that returns memory back to Geany from `g_*` to `malloc` means the frees inside Geany need to be changed from `g_free()` to` free()` since you can't mix them (according to the Glib [description section](https://developer.gnome.org/glib/stable/glib-Memory-Allocation.html)) It is going to be confusing if some memory in geany needs to be `g_free`d and some plain `free`d. And I hope none of that gets to the plugin API.

2. `g_strerror()` is different to POSIX in that its guaranteed UTF-8 whereas POSIX is locale dependent. That means it needs conversion if its to be displayed in the UI. OT not *all* `g_` string [changes](https://github.com/geany/geany/commit/084c23bbb163fd95d00e2032698ec3371d3666...) were part of a global change :grin:

3. I believe `g_stat` etc work around much windows weirdness, which would have to be solved if the Glib version was not used. (BTW why don't these get #605 type problems?)

"HTML parser? I'm too busy generating the [expletive deleted] stuff" which is why I don't have much Geany time lately. Use the expat library.

IRO the new uctags parser: I understand it is intended to do local variables, but it didn't seem to provide them when I tried, and anyway what about scope for locals, and Geany does not understand scopes. It would be a real mess if all locals were offered for completions all the time.

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160#issuecomment-237524652

...

So I hope our poor old @b4n won't suffer much

he only became old through reviewing patch sets from @techee and @kugel- 😀

Very true. I take full responsibility for his grey hair :-)

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160#issuecomment-237533126

regarding whitespace changes: I didn't give it a shot yet, but normally Git should be very useful to review those. If they are only indentation changes, `git show -b` should show an empty diff. And oddly enough I'd trust Git :)

Regarding memory allocation variants: I don't care so long as as @elextr the deallocator matches (even though nowadays GLib's will always match libc's, it'd be poor design to mix them). But remember that GLib's abort on allocation failure, while libc's just return NULL. I think CTags has wrapper for that itself, but it's an important difference.

About regex, well… the problem is that we'd need a bundled version of GNU regex, and that would mean keeping it up to date. If you really wanna do that, maybe "just" write a tiny wrapper using GRegex in a compatible API? I'm kinda afraid of having to keep a copy of GNU regex -- and it's especially ironic when it's in the process of syncing another lib :)

Regarding the HTML parser, yeah a non-regex one would probably be better, and indeed fairly easy to write. UCTags has even added libxml-based parsers, so maybe it could use that if we didn't care about the dep (FWIW, we already indirectly link to libexpat).

That said, let's get to the meat of this.

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160#issuecomment-237545634

@elextr @techee see, 6bc579798716dbe94a171a016f9989c92c4b67e2 was quicker to review than some previous patches 'cause `git show -b 6bc579798716dbe94a171a016f9989c92c4b67e2` was very straightforward :) (about 62 lines changes (vim settings), instead of about 7446)

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160#issuecomment-237549978

About regex, well… the problem is that we'd need a bundled version of GNU regex, and that would mean keeping it up to date. If you really wanna do that, maybe "just" write a tiny wrapper using GRegex in a compatible API? I'm kinda afraid of having to keep a copy of GNU regex -- and it's especially ironic when it's in the process of syncing another lib :)

I know ctags has a bundled GNU regex library but I think it only falls back to it when GNU regex isn't present in the system (the native windows msvc build I guess). But for Windows we use msys2 where I'd suspect it would be present and the Mac build uses jhbuild to build all typical linux dependencies so I think it's there too. So do we actually need to bundle GNU regex?

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160#issuecomment-237552119

• 	mio_getpos (mio, &startOfLine);
  

• 	startOfLine = mio_tell(fp);
  

how comes this is different in UCTags? But well, whatever, practically it's the same.

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160/files/bbbbb309baa0ff6a94f50437e99cc...

@elextr @techee see, 6bc5797 was quicker to review than some previous patches 'cause git show -b 6bc579798716dbe94a171a016f9989c92c4b67e2 was very straightforward :) (about 62 lines changes (vim settings), instead of about 7446)

I could have smuggled in some evil indents which you don't see this way :-P. In fact there are some indent diffs compared to uctags - Geany always replaces tabs with spaces but it replaces spaces with tabs only at the beginning of a line:

https://github.com/geany/geany/blob/master/src/editor.c#L4484 (based on the comment in the corresponding commit it's probably OK).

This means that e.g. between

``` some_code(); /* some comment */ ```

the whitespace used is incorrect. But the number of these is much smaller and these can get fixed on the way.

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160#issuecomment-237558212

• 	mio_getpos (mio, &startOfLine);
  

• 	startOfLine = mio_tell(fp);
  

When I was adding MIO to uctags I found it easier to replace FILE with MIO and just follow the errors and fix them on the way (my way) than checking how it's done in Geany. So there are minor differences like that.

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160/files/bbbbb309baa0ff6a94f50437e99cc...

Well, yeah, but you didn't, did you? :)

And well, now I also checked you didn't alter the content of any string. So as you only touched C files, and C is whitespace-agnostic, the worse you could have done is ugly misleading indentation.

Right. That's why I added all the spyware code in https://github.com/geany/geany/pull/1160/commits/ea72ecc00fc22f24eced3ffce7c... :-)

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160#issuecomment-237565237

tag->extensionFields.signature = arglist;
tag->extensionFields.varType = vartype;

}

• else

• initTagEntry (tag, NULL);
  

I'm afraid it's not really possible to just not initialize the entry, is it?

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160/files/359c60b81b8d80af4551f638b2b8c...

result->fileKind = &defaultFileKind;

• result->enabled = TRUE;

hum, how comes this gets added here?

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160/files/781f4ef1caba2c0c3fbaa5dbc9481...

I'm at 8c0bfbb0f388124f601049301e2742bb1d7ccf1b, but I take a little break.

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160#issuecomment-237576255

I'm at 8c0bfbb, but I take a little break.

Well deserved, quite a headache source isn't it?

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160#issuecomment-237577143

Yeah totally, I'm trying to stop before my head explodes :]

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160#issuecomment-237577524

+extern boolean isDestinationStdout (void) +{

• boolean toStdout = FALSE;
• if (Option.xref || Option.filter ||

• (Option.tagFileName != NULL  &&  (strcmp (Option.tagFileName, "-") == 0
  

• 				  || strcmp (Option.tagFileName, "/dev/stdout") == 0
  

• )))
  

• toStdout = TRUE;
  

• return toStdout;

+}

+# if defined (WIN32)

+static boolean createTagsForMatchingEntries (char *const pattern)

this was #if0'd in ctags.c

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160/files/d440a81166e62d41e2b06d555b471...

• if (totalTags > 0 && Option.sorted)
• {

• fprintf (errout, "%lu tag%s sorted", totalTags, plural (totalTags));
  

+#ifdef CLOCK_AVAILABLE

• fprintf (errout, " in %.02f seconds",
  

• 		((double) (timeStamps [2] - timeStamps [1])) / CLOCKS_PER_SEC);
  

+#endif

• fputc ('\n', errout);
  

• }

+#ifdef TM_DEBUG

• fprintf (errout, "longest tag line = %lu\n",

• 	(unsigned long) TagFile.max.line);
  

+#endif +}

actually, all 'til here

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160/files/d440a81166e62d41e2b06d555b471...

+/* Hack for ridiculous practice of Microsoft Visual C++.

• */

+#if defined (WIN32) +# if defined (_MSC_VER) +# define stat _stat +# define getcwd _getcwd +# define PATH_MAX _MAX_PATH +# endif +#endif

+#ifndef PATH_MAX +# define PATH_MAX 256 +#endif

+#define selected(var,feature) (((int)(var) & (int)(feature)) == (int)feature)

should go below the *miscellaneous macros* comment below

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160/files/d440a81166e62d41e2b06d555b471...

+# endif +#endif

+#ifdef HAVE_DIRECT_H +# include <direct.h> /* to _getcwd */ +#endif +#ifdef HAVE_DIR_H +# include <dir.h> /* to declare findfirst() and findnext() */ +#endif +#ifdef HAVE_IO_H +# include <io.h> /* to declare open() */ +#endif +#include "debug.h" +#include "routines.h" +#ifdef HAVE_ICONV +# include "mbcs.h"

well, if it's properly guarded like that it doesn't matter much

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160/files/d440a81166e62d41e2b06d555b471...

+# endif +#endif

+#ifdef HAVE_DIRECT_H +# include <direct.h> /* to _getcwd */ +#endif +#ifdef HAVE_DIR_H +# include <dir.h> /* to declare findfirst() and findnext() */ +#endif +#ifdef HAVE_IO_H +# include <io.h> /* to declare open() */ +#endif +#include "debug.h" +#include "routines.h" +#ifdef HAVE_ICONV +# include "mbcs.h"

Apparently it's the only usage of `HAVE_ICONV` in Geany apparently, the other haven't been imported from U-CTags.

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160/files/d440a81166e62d41e2b06d555b471...

+}

+extern boolean isDestinationStdout (void) +{

• boolean toStdout = FALSE;
• if (Option.xref || Option.filter ||

• (Option.tagFileName != NULL  &&  (strcmp (Option.tagFileName, "-") == 0
  

• 				  || strcmp (Option.tagFileName, "/dev/stdout") == 0
  

• )))
  

• toStdout = TRUE;
  

• return toStdout;

+}

+# if defined (WIN32)

shouldn't probably have the space after the `#` as IIUC it's top level

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160/files/d440a81166e62d41e2b06d555b471...

+extern boolean isDestinationStdout (void) +{

• boolean toStdout = FALSE;
• if (Option.xref || Option.filter ||

• (Option.tagFileName != NULL  &&  (strcmp (Option.tagFileName, "-") == 0
  

• 				  || strcmp (Option.tagFileName, "/dev/stdout") == 0
  

• )))
  

• toStdout = TRUE;
  

• return toStdout;

+}

+# if defined (WIN32)

+static boolean createTagsForMatchingEntries (char *const pattern)

was #if0'd

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160/files/d440a81166e62d41e2b06d555b471...

• strcpy (CurrentDirectory, cwd);
  

• else

• sprintf (CurrentDirectory, "%s%c", cwd, OUTPUT_PATH_SEPARATOR);
  

• free (cwd);

+} +#endif

+extern boolean doesFileExist (const char *const fileName) +{

• GStatBuf fileStatus;
• return (boolean) (g_stat (fileName, &fileStatus) == 0);

+}

+extern boolean isRecursiveLink (const char* const dirName)

this was #if0'd, and the impl in UCTags is different (but this one matches the old one we had, so I guess it's expected)

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160/files/d440a81166e62d41e2b06d555b471...

+# else +# define PATH_SEPARATOR '/' +# endif +#endif

+#if defined (MSDOS_STYLE_PATH) && defined (UNIX_PATH_SEPARATOR) +# define OUTPUT_PATH_SEPARATOR '/' +#else +# define OUTPUT_PATH_SEPARATOR PATH_SEPARATOR +#endif

+/* +* DATA DECLARATIONS +*/ +#if defined (MSDOS_STYLE_PATH) +extern const char *const PathDelimiters;

that's not the same as in the impl, and the impl is `static`, which is a bit contradictory ^^

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160/files/d440a81166e62d41e2b06d555b471...

Okay, I think I'm done for now :)

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160#issuecomment-237997281

• 		vStringCatS (filePath, entryName);
  

• 		resize |= createTagsForEntry (vStringValue (filePath));
  

• 	}
  

• } while (_findnext (hFile, &fileInfo) == 0);
  

• _findclose (hFile);
  

• }

+#endif

• vStringDelete (filePath);
• return resize;

+}

+#endif

+static boolean recurseIntoDirectory (const char *const dirName)

Should I #if0 it or keep it the way it is? There will always be some dead code and the few lines here won't help much I think.

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160/files/d440a81166e62d41e2b06d555b471...

+static const char *ExecutableName = "geany";

+/* +* FUNCTION PROTOTYPES +*/ +#ifdef NEED_PROTO_STAT +extern int stat (const char *, struct stat *); +#endif +#ifdef NEED_PROTO_LSTAT +extern int lstat (const char *, struct stat *); +#endif

+#if 0 +static boolean createTagsForEntry (const char *const entryName); +#endif

This one got removed too.

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160/files/d440a81166e62d41e2b06d555b471...

+# else +# define PATH_SEPARATOR '/' +# endif +#endif

+#if defined (MSDOS_STYLE_PATH) && defined (UNIX_PATH_SEPARATOR) +# define OUTPUT_PATH_SEPARATOR '/' +#else +# define OUTPUT_PATH_SEPARATOR PATH_SEPARATOR +#endif

+/* +* DATA DECLARATIONS +*/ +#if defined (MSDOS_STYLE_PATH) +extern const char *const PathDelimiters;

In Geany it was static and in uctags it's extern. Apparently I kept the static from Geany in the impl and took uctags for the header and this was the result.

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160/files/d440a81166e62d41e2b06d555b471...

Okay, I think I'm done for now :)

Fear not, I'll keep you saturated :-).

I committed fixes for the various issues (hopefully all that matter). In addition, I tried to sync some more trivial stuff from parsers so there's some whitespace sync in parsers, some more function renames and similar.

I also pushed my changes to uctags here

https://github.com/techee/ctags/tree/geany_sync

where I'll be adding things that should go from Geany to ctags. When diffing it's best to compare to this branch to see less diffs.

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160#issuecomment-238093461

@techee pushed 1 commit.

e866a97 entry: Move functions around a bit to reduce the amount of diffs

--- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/1160/files/ca65fa22367042ea8c88efab7e76d...

• 		vStringCatS (filePath, entryName);
  

• 		resize |= createTagsForEntry (vStringValue (filePath));
  

• 	}
  

• } while (_findnext (hFile, &fileInfo) == 0);
  

• _findclose (hFile);
  

• }

+#endif

• vStringDelete (filePath);
• return resize;

+}

+#endif

+static boolean recurseIntoDirectory (const char *const dirName)

(well, even: if it's in UCTags, better keep it the closest, so not #if0ing it)

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160/files/d440a81166e62d41e2b06d555b471...

+# endif +#endif

+#ifdef HAVE_DIRECT_H +# include <direct.h> /* to _getcwd */ +#endif +#ifdef HAVE_DIR_H +# include <dir.h> /* to declare findfirst() and findnext() */ +#endif +#ifdef HAVE_IO_H +# include <io.h> /* to declare open() */ +#endif +#include "debug.h" +#include "routines.h" +#ifdef HAVE_ICONV +# include "mbcs.h"

I don't know how realistic it is that HAVE_ICONV will be present but we can add something like

``` #ifdef HAVE_ICONV # undef HAVE_ICONV #endif ```

to general.h to be sure. What do you think?

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160/files/d440a81166e62d41e2b06d555b471...

Apart from possible fixes I don't plan much more for this pull request. Once it is merged, I'll start working on some more "main" syncs. I'll probably first create a pull request with the "easy" things which is about everything except parse.c and lregex.c which I'll leave at the very end in a separate pull request.

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160#issuecomment-238390939

+# endif +#endif

+#ifdef HAVE_DIRECT_H +# include <direct.h> /* to _getcwd */ +#endif +#ifdef HAVE_DIR_H +# include <dir.h> /* to declare findfirst() and findnext() */ +#endif +#ifdef HAVE_IO_H +# include <io.h> /* to declare open() */ +#endif +#include "debug.h" +#include "routines.h" +#ifdef HAVE_ICONV +# include "mbcs.h"

Hum… is general.h not going to get merged? I mean, this is the current only use of this macro, and if this file isn't added an the macro not used in other synced code, I don't see why have this some place else than here.

But anyway, it probably doesn't matter much.

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160/files/d440a81166e62d41e2b06d555b471...

+# endif +#endif

+#ifdef HAVE_DIRECT_H +# include <direct.h> /* to _getcwd */ +#endif +#ifdef HAVE_DIR_H +# include <dir.h> /* to declare findfirst() and findnext() */ +#endif +#ifdef HAVE_IO_H +# include <io.h> /* to declare open() */ +#endif +#include "debug.h" +#include "routines.h" +#ifdef HAVE_ICONV +# include "mbcs.h"

Hum… is general.h not going to get merged? I mean, this is the current only use of this macro, and if this file isn't added an the macro not used in other synced code, I don't see why have this some place else than here.

We'll have some diffs against uctags e.g. to install our tag hook anyway so this could be one of the additional differences. In uctags #ifdef HAVE_ICONV is checked at quite many places so it's probably better to have it undefined once in general.h than having to do something at every place it appears.

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160/files/d440a81166e62d41e2b06d555b471...

+# endif +#endif

+#ifdef HAVE_DIRECT_H +# include <direct.h> /* to _getcwd */ +#endif +#ifdef HAVE_DIR_H +# include <dir.h> /* to declare findfirst() and findnext() */ +#endif +#ifdef HAVE_IO_H +# include <io.h> /* to declare open() */ +#endif +#include "debug.h" +#include "routines.h" +#ifdef HAVE_ICONV +# include "mbcs.h"

IMO it shouldn't be defined unless someone does something really strange so we shouldn't need to do anything special. But I'll leave the decision up to you.

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160/files/d440a81166e62d41e2b06d555b471...

+# endif +#endif

+#ifdef HAVE_DIRECT_H +# include <direct.h> /* to _getcwd */ +#endif +#ifdef HAVE_DIR_H +# include <dir.h> /* to declare findfirst() and findnext() */ +#endif +#ifdef HAVE_IO_H +# include <io.h> /* to declare open() */ +#endif +#include "debug.h" +#include "routines.h" +#ifdef HAVE_ICONV +# include "mbcs.h"

I don't know how realistic it is that HAVE_ICONV will be present but we can add something like [...]

No need to do `#ifdef` first, it's perfectly legal to `#undef` a macro that isn't defined (C99 6.10.3.5).

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160/files/d440a81166e62d41e2b06d555b471...

+# endif +#endif

+#ifdef HAVE_DIRECT_H +# include <direct.h> /* to _getcwd */ +#endif +#ifdef HAVE_DIR_H +# include <dir.h> /* to declare findfirst() and findnext() */ +#endif +#ifdef HAVE_IO_H +# include <io.h> /* to declare open() */ +#endif +#include "debug.h" +#include "routines.h" +#ifdef HAVE_ICONV +# include "mbcs.h"

I indeed don't think it's valid to just define HAVE_ICONV and whine it breaks

Agree, since it appears to not be in public (TM) headers. If it was, I could imagine a plugin defining it if it used libiconv and causing trouble.

--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1160/files/d440a81166e62d41e2b06d555b471...

@techee pushed 1 commit.

d0cc3dc Remove TagEntryFunction check in c.c

--- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/1160/files/0eca258e58693ebdf0670b74e6934...

@techee pushed 1 commit.

a974f35 Define KEYWORD_NONE in keyword.h so it doesn't have to be defined by parsers

@techee pushed 1 commit.

b7f7ce2 Drop vi modelines

b7f7ce267546d4e43511b613594daeb6e8051559 OK -- although U-CTags removed them because they added editorconfig config file IIUC.

cb7da79824ccd023c495de37f047ae1441b45fd5 OK

@b4n Done. (I was done something like 12 days ago, committed the few extra patches just because the pull request was still open and the patches made sense for it.)

``` ../../ctags/parsers/c.c: In function 'processToken': ../../ctags/parsers/c.c:2033:3: warning: case value '4294967295' not in enumerated type 'keywordId {aka enum eKeywordId}' [-Wswitch] case KEYWORD_NONE: processName (st); break; ^~~~ ``` How scared should we be? that's a typical signed/unsigned issue, but hum.

Otherwise, should be mergeable indeed.

@techee pushed 3 commits.

1e14667 Don't compare foreign values to enumeration type f511787 Add missing prototypes 63c054a #if 0 currently unused code to avoid compiler warning

Merged #1160.