[Github-comments] [geany/geany] Sync ctags main part (#1263)

[Colomban Wendling]

b4n commented on this pull request.



>  {
-	const char *p = parameter;
-	bool mode = true;
-	int c;
+	bool tagFileResized;
+	pushNarrowedInputStream (language,
+				 startLine, startCharOffset,
+				 endLine, endCharOffset,
+				 sourceLineOffset);
+#ifndef CTAGS_LIB
+	tagFileResized = createTagsWithFallback1 (language);
+#endif
+	popNarrowedInputStream  ();
+	return tagFileResized;

> > In runParserInNarrowedInputStream(): shouldn't more be guarded? Here the meat of the function is disabled with CTAGS_LIB, but not all.
>
>What else do you think should be guarded? The rest seems to just manipulate the input file's MIO which shouldn't be a problem I think.

My point was that then the whole manipulation looks like a no-op, and so if guarding anything, the whole body could be guarded.

BTW, I see that now it returns a uninitialized value (`tagFileResized`).

>  {
-	const char *p = parameter;
-	bool mode = true;
-	int c;
+	bool tagFileResized;
+	pushNarrowedInputStream (language,
+				 startLine, startCharOffset,
+				 endLine, endCharOffset,
+				 sourceLineOffset);
+#ifndef CTAGS_LIB
+	tagFileResized = createTagsWithFallback1 (language);
+#endif
+	popNarrowedInputStream  ();
+	return tagFileResized;

BTW-2, isn't this needed to run sub-parsers?

> +
+#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 (" ");

confusing indent for L36 and L34 as they are guarded by the `if` on L32

> @@ -174,6 +175,23 @@ extern void cppUngetc (const int c)
 	Assert (Cpp.ungetch2 == '\0');
 	Cpp.ungetch2 = Cpp.ungetch;
 	Cpp.ungetch = c;
+	if (collectingSignature)
+		vStringChop (signature);
+}
+
+static inline int getcAndCollect ()

Picky: `(void)` for the arglist

>  	if (! tag->kind->enabled)
 		return;
+#endif

Maybe instead of removing this altogether the library consumer could have to enable kinds he cares about?  Not sure, as I tend to agree with you that post-filtering seems better, but maybe filtering ahead has other advantages like performance.  Anyway, looks good enough for us, but could make sense to keep in a final lib API.

> @@ -20,5 +20,5 @@ obj
 qar�64�Struct.Union�0�int
 quxx�64�Struct.Union�0�bool
 test.simple�256�0
-tfun�16�(T)�Class�0�auto
+tfun�16�(T v)�Class�0�auto

The source is valid D code according to GDC.  And from what I can guess, the signature is more likely to be `(T v)` than `(T)` (as `v` is used as a variable in the body); I'd guess the `(T)` part is similar to C++'s `<T>` in `tfun<T>(T v)`.

So I'd say this change is even better.

-- 
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/1263#pullrequestreview-146762584
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.geany.org/pipermail/github-comments/attachments/20180820/65f1fa95/attachment-0001.html>