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 790345 - Multiple improvements to cmake.lang
Multiple improvements to cmake.lang
Status: RESOLVED FIXED
Product: gtksourceview
Classification: Platform
Component: Syntax files
git master
Other All
: Normal enhancement
: ---
Assigned To: GTK Sourceview maintainers
GTK Sourceview maintainers
Depends on:
Blocks:
 
 
Reported: 2017-11-14 21:34 UTC by Роман Донченко
Modified: 2017-11-16 14:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patchset (45.39 KB, patch)
2017-11-14 21:34 UTC, Роман Донченко
none Details | Review
cmake.lang: update the lists of builtins to match CMake 3.10.0-rc5 (22.95 KB, patch)
2017-11-15 22:14 UTC, Роман Донченко
committed Details | Review
cmake.lang: remove the "builtin-variable-2" context (1.53 KB, patch)
2017-11-15 22:14 UTC, Роман Донченко
committed Details | Review
cmake.lang: multiple improvements to the "variable" context (2.67 KB, patch)
2017-11-15 22:14 UTC, Роман Донченко
committed Details | Review
cmake.lang: add highlighting for environment variable references (2.57 KB, patch)
2017-11-15 22:14 UTC, Роман Донченко
committed Details | Review
cmake.lang: add highlighting for quoted arguments (2.13 KB, patch)
2017-11-15 22:14 UTC, Роман Донченко
committed Details | Review
cmake.lang: add highlighting for bracket arguments (1.87 KB, patch)
2017-11-15 22:14 UTC, Роман Донченко
committed Details | Review
cmake.lang: add highlighting for bracket comments (1.96 KB, patch)
2017-11-15 22:14 UTC, Роман Донченко
committed Details | Review
cmake.lang: highlight escape seqs and legacy syntax in unquoted args (3.19 KB, patch)
2017-11-15 22:14 UTC, Роман Донченко
committed Details | Review
cmake.lang: make the top-level context more strict (2.51 KB, patch)
2017-11-15 22:15 UTC, Роман Донченко
committed Details | Review
cmake.lang: recognize unset() as a variable assignment (969 bytes, patch)
2017-11-15 22:15 UTC, Роман Донченко
committed Details | Review
cmake.lang: clean up the "operator" and "constant" contexts (1.76 KB, patch)
2017-11-15 22:15 UTC, Роман Донченко
committed Details | Review
cmake.lang: add a test file (1.79 KB, patch)
2017-11-15 22:15 UTC, Роман Донченко
committed Details | Review

Description Роман Донченко 2017-11-14 21:34:36 UTC
Created attachment 363642 [details] [review]
Proposed patchset

I'm attaching a patchset that improves CMake syntax highlighting in various ways and adds a test file. The details are listed in the individual commit messages.
Comment 1 Sébastien Wilmet 2017-11-15 12:50:08 UTC
To be able to review the commits here, you need to attach each commit separately.

It is normally planned to move to the GNOME GitLab instance in the near future (maybe in a few months), so it'll ease contribution with a pull-request workflow. But in the meantime, it is still with patches attached to bugzilla. If you often need to attach patches to bugzilla, it is easier to use git-bz:
http://git.fishsoup.net/cgit/git-bz/
Comment 2 Роман Донченко 2017-11-15 22:14:21 UTC
Created attachment 363767 [details] [review]
cmake.lang: update the lists of builtins to match CMake 3.10.0-rc5
Comment 3 Роман Донченко 2017-11-15 22:14:26 UTC
Created attachment 363768 [details] [review]
cmake.lang: remove the "builtin-variable-2" context

LOCATION is a target property, while the rest of the keywords in it are
keywords used by the add_custom_command command. Not only are these keywords
selected seemingly arbitrarily, but none of them are builtin variables,
so there's no reason for them to be given the "builtin-variable" style.
Comment 4 Роман Донченко 2017-11-15 22:14:31 UTC
Created attachment 363769 [details] [review]
cmake.lang: multiple improvements to the "variable" context

* rename to "variable-reference", since that's the official name of the
  corresponding syntax element;
* add support for nesting;
* apply the "special char" style to the ${} markers;
* highlight escape sequences in variable names;
* highlight disallowed characters in variable names.
Comment 5 Роман Донченко 2017-11-15 22:14:36 UTC
Created attachment 363770 [details] [review]
cmake.lang: add highlighting for environment variable references

The syntax is nearly the same as for regular variable references, and
I would've just merged the contexts, but builtin variable names have
no special meaning in environment variable references, so I opted to
use a separate context to implement that distinction.

There are actually special environment variable names that CMake
recognizes, but I haven't implemented support for them.
Comment 6 Роман Донченко 2017-11-15 22:14:41 UTC
Created attachment 363771 [details] [review]
cmake.lang: add highlighting for quoted arguments
Comment 7 Роман Донченко 2017-11-15 22:14:46 UTC
Created attachment 363772 [details] [review]
cmake.lang: add highlighting for bracket arguments
Comment 8 Роман Донченко 2017-11-15 22:14:51 UTC
Created attachment 363773 [details] [review]
cmake.lang: add highlighting for bracket comments
Comment 9 Роман Донченко 2017-11-15 22:14:56 UTC
Created attachment 363774 [details] [review]
cmake.lang: highlight escape seqs and legacy syntax in unquoted args
Comment 10 Роман Донченко 2017-11-15 22:15:01 UTC
Created attachment 363775 [details] [review]
cmake.lang: make the top-level context more strict

Now commands are no longer highlighted in argument lists, nor is
argument syntax highlighted outside argument lists.
Comment 11 Роман Донченко 2017-11-15 22:15:06 UTC
Created attachment 363776 [details] [review]
cmake.lang: recognize unset() as a variable assignment
Comment 12 Роман Донченко 2017-11-15 22:15:11 UTC
Created attachment 363777 [details] [review]
cmake.lang: clean up the "operator" and "constant" contexts

CACHE and BOOL are clearly not constants, but they _are_ keywords used
by the set command, so move them to "operator". Several boolean constants
recognized by the if command are missing from "constant", so add them.
Comment 13 Роман Донченко 2017-11-15 22:15:16 UTC
Created attachment 363778 [details] [review]
cmake.lang: add a test file
Comment 14 Sébastien Wilmet 2017-11-16 14:42:33 UTC
Review of attachment 363767 [details] [review]:

Looks good.
Comment 15 Sébastien Wilmet 2017-11-16 14:43:04 UTC
Review of attachment 363768 [details] [review]:

Looks good.
Comment 16 Sébastien Wilmet 2017-11-16 14:46:47 UTC
You seem to know well cmake, it's a nice patchset, since I don't know well cmake I haven't done a detailed review. Let's merge this.
Comment 17 Sébastien Wilmet 2017-11-16 14:49:33 UTC
Thanks for your contribution!

Attachment 363767 [details] pushed as 6877b91 - cmake.lang: update the lists of builtins to match CMake 3.10.0-rc5
Attachment 363768 [details] pushed as 2c63846 - cmake.lang: remove the "builtin-variable-2" context
Attachment 363769 [details] pushed as 4007ba0 - cmake.lang: multiple improvements to the "variable" context
Attachment 363770 [details] pushed as c67b869 - cmake.lang: add highlighting for environment variable references
Attachment 363771 [details] pushed as f030c3a - cmake.lang: add highlighting for quoted arguments
Attachment 363772 [details] pushed as 533f5a3 - cmake.lang: add highlighting for bracket arguments
Attachment 363773 [details] pushed as 2eed8cb - cmake.lang: add highlighting for bracket comments
Attachment 363774 [details] pushed as 038b60d - cmake.lang: highlight escape seqs and legacy syntax in unquoted args
Attachment 363775 [details] pushed as dd4e814 - cmake.lang: make the top-level context more strict
Attachment 363776 [details] pushed as 648a76a - cmake.lang: recognize unset() as a variable assignment
Attachment 363777 [details] pushed as dde1d7b - cmake.lang: clean up the "operator" and "constant" contexts
Attachment 363778 [details] pushed as 9479b44 - cmake.lang: add a test file