After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 748960 - Ocaml highlighter does not close empty comments (**)
Ocaml highlighter does not close empty comments (**)
Status: RESOLVED FIXED
Product: gtksourceview
Classification: Platform
Component: Syntax files
git master
Other All
: Normal minor
: ---
Assigned To: GTK Sourceview maintainers
GTK Sourceview maintainers
Depends on:
Blocks:
 
 
Reported: 2015-05-05 14:59 UTC by Gaetan Gilbert
Modified: 2015-05-10 18:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fixes the bug. (799 bytes, patch)
2015-05-05 14:59 UTC, Gaetan Gilbert
none Details | Review
Ocaml example (741 bytes, patch)
2015-05-09 11:35 UTC, Gaetan Gilbert
accepted-commit_now Details | Review
Indentation fix (893 bytes, patch)
2015-05-09 11:35 UTC, Gaetan Gilbert
none Details | Review
Intended highlighting for example (7.50 KB, image/png)
2015-05-09 11:36 UTC, Gaetan Gilbert
  Details
Lookahead regex fix (831 bytes, patch)
2015-05-10 18:32 UTC, Gaetan Gilbert
accepted-commit_now Details | Review

Description Gaetan Gilbert 2015-05-05 14:59:45 UTC
Created attachment 302933 [details] [review]
Fixes the bug.

In ocaml, (**) is a closed comment but the highlighter matches the (** as the beginning of an ocamldoc section.
Comment 1 Sébastien Wilmet 2015-05-09 10:13:17 UTC
Review of attachment 302933 [details] [review]:

Thanks for the patch.

The indentation is wrong. It must normally be two spaces.

ocamldoc is defined like this:
<context id="ocamldoc" style-ref="ocamldoc">
  <start>\(\*\*</start>
  <end>\*\)</end>

Why (**) is matched by ocamldoc? I see how (***) matches ocamldoc, but for (**) I don't see why. Unless <start> and <end> can overlap.

Can you provide a code example in tests/syntax-highlighting/ ? With an example of a normal comment, an empty comment and an ocamldoc. So we can test more easily the change.
Comment 2 Gaetan Gilbert 2015-05-09 11:34:44 UTC
The (** part is matched by the ocamldoc start, and then the end doesn't match so everything after it is highlighted wrong.
See patch and image.
Comment 3 Gaetan Gilbert 2015-05-09 11:35:29 UTC
Created attachment 303119 [details] [review]
Ocaml example
Comment 4 Gaetan Gilbert 2015-05-09 11:35:57 UTC
Created attachment 303120 [details] [review]
Indentation fix
Comment 5 Gaetan Gilbert 2015-05-09 11:36:37 UTC
Created attachment 303121 [details]
Intended highlighting for example
Comment 6 Paolo Borelli 2015-05-09 13:30:37 UTC
Review of attachment 303120 [details] [review]:

Instead of adding a separate context, maybe we can use regex look-ahead in the ocaml doc definition so that it matches (** not followed by )
Comment 7 Gaetan Gilbert 2015-05-10 13:52:16 UTC
I don't know how to do that.
Comment 8 Sébastien Wilmet 2015-05-10 17:19:07 UTC
See the section "Lookahead assertions" at:

https://developer.gnome.org/glib/stable/glib-regex-syntax.html

(you can also see that page in devhelp, if the glib documentation is installed.)
Comment 9 Sébastien Wilmet 2015-05-10 17:21:04 UTC
Review of attachment 303119 [details] [review]:

Looks good.
Comment 10 Sébastien Wilmet 2015-05-10 17:23:03 UTC
Review of attachment 303120 [details] [review]:

You can also squash the indentation fix with the previous commit, so that there is only one commit.
You can do that with git rebase -i, or directly with git commit --amend (if the second commit is not already created).
Comment 11 Gaetan Gilbert 2015-05-10 18:32:52 UTC
Created attachment 303190 [details] [review]
Lookahead regex fix
Comment 12 Sébastien Wilmet 2015-05-10 18:46:11 UTC
Review of attachment 303190 [details] [review]:

Thanks, looks good.
Comment 13 Sébastien Wilmet 2015-05-10 18:50:05 UTC
Commits pushed to the master branch:

https://git.gnome.org/browse/gtksourceview/commit/?id=08f527445925d191ca3cf23e1984cfcc5c8d620f

https://git.gnome.org/browse/gtksourceview/commit/?id=5f265ef956496c4fcf0ff8d8a322be1c2876d9b5

I just added the bug URL at the end of the commit messages.