GNOME Bugzilla – Bug 689354
no longer parses comments which don't end with */ on a newline
Last modified: 2015-02-07 16:52:46 UTC
Originated from https://bugzilla.gnome.org/show_bug.cgi?id=688897 This at least broke the ibus build: http://git.gnome.org/browse/gnome-ostree/commit/?id=709ed2920f74394b6c732036825ef2c611f2303f If it's just this one comment in the ibus source, we could probably get away with this change. Otherwise, we should consider parsing it (and emitting a warning).
Created attachment 230290 [details] [review] scanner: Parse comments with */ not on a new line, but emit a warning We don't know how many apps do this, but at least ibus had one.
Review of attachment 230290 [details] [review]: Other than the minor nitpick, great catch, thanks :) ::: giscanner/annotationparser.py @@ +170,3 @@ +BROKEN_COMMENT_END_RE = re.compile(r''' + [^\S\n\r]* # 0 or more whitespace characters + \*+ # 1 or more asterisk characters Here we correctly check for 1 or more "*" @@ +818,3 @@ del comment_lines[-1] + elif BROKEN_COMMENT_END_RE.search(lastline): + without_trailing_comment_end = lastline.replace('*/', '') But here we only replace on one "*". ::: tests/scanner/annotationparser/gi/syntax.xml @@ +96,3 @@ + <name>Test</name> + </identifier> + <description>something *</description> so that "*" should probably not be part of the description, a comment block ending with: """ where something is done. **/ """ would end up looking weird in the docs: """ where something is done. * """
Review of attachment 230290 [details] [review]: ::: giscanner/annotationparser.py @@ +819,3 @@ + elif BROKEN_COMMENT_END_RE.search(lastline): + without_trailing_comment_end = lastline.replace('*/', '') + comment_lines[-1] = (comment_lines[1][0], without_trailing_comment_end) That probably should be comment_line[-1][0], or warnings will point to the line number at the beginning of the comment block instead of the last line. Sorry for missing it before, must have been too tired :/
Created attachment 230386 [details] [review] scanner: Parse comments with */ not on a new line, but emit a warning How about something like this?
Review of attachment 230386 [details] [review]: One comment: ::: giscanner/annotationparser.py @@ +815,3 @@ + comment_lines[-1] = (line_offset, description) + position = message.Position(filename, lineno + line_offset) + marker = ' '*result.end('description') + '^' Not sure that we should attempt to invent clang-style errors here. I mean, it is nicer, but if for example the terminal width is smaller than the source line, your code here won't work. To do clang-style right, the output code needs to do isatty(2) and format appropriately based on COLUMNS (including possibly ellipsizing the original text) This could be done inside message.warn() with the current API, I think.
(In reply to comment #5) > Not sure that we should attempt to invent clang-style errors here. Was more looking at emulating the way some parts of GTK-Doc complain when writing those (think it's when building html and even then they are really coming from xsltproc, not sure). > I mean, it > is nicer, but if for example the terminal width is smaller than the source > line, your code here won't work. Ah, good point, didn't consider terminals (have my build output go into text files, thank you very much). Guess this applies to all other warnings recently added to annotationparser.py too then. > To do clang-style right, the output code needs to do isatty(2) and format > appropriately based on COLUMNS (including possibly ellipsizing the original > text) This could be done inside message.warn() with the current API, I think. How important is this? Can we put it in it's own bug report and revisit it when the rest of the GTK-Doc parser work is done?
(In reply to comment #6) > Ah, good point, didn't consider terminals (have my build output go > into text files, thank you very much). Guess this applies to all other > warnings recently added to annotationparser.py too then. Ah...I didn't see you'd done this elsewhere. Ok yes, let's go with the patch you have now, and open a further bug for doing "caret errors" in the lower levels of the message system.
https://bugzilla.gnome.org/show_bug.cgi?id=689454
Review of attachment 230386 [details] [review]: Ok.
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]