D7175: Redesign CMake syntax

[Dominik Haumann]

dhaumann added a comment.


  Sure? It was reverted a month ago once. Was it applied again?

REVISION DETAIL
  https://phabricator.kde.org/D7175

To: turbov, dhaumann, #kate, #framework_syntax_highlighting, vkrause, cullmann
Cc: cullmann, #frameworks, michaelh, ngraham, bruns

D7175: Redesign CMake syntax

[Christoph Cullmann]

cullmann closed this revision.
cullmann added a comment.


  This is already pushed ;=)
  
  Autor: Alex Turbov   2017-08-07 08:01:46
  Eintragender: Alex Turbov   2018-03-28 21:17:34
  Eltern: aa5a8e08e0a2004536ab87531ef93217cf4a 
 
(GIT_SILENT Upgrade Qt5 version requirement to 5.8.0.)
  Kind:  683fc509a1ad5ac36e2446cf35a123f479455636 
 
(Make it possible to fully build the project when crosscompiling)
  Zweig: master, remotes/origin/master
  Folgt auf: v5.30.0-rc1
  Vorgänger von: v5.45.0, v5.45.0-rc1
  
Redesign CMake syntax generator

The new approach is to use a trivial Python script, which uses a prepared 
YAML
data file and Jinja template to render the resulting syntax file.
Key features of the new implementation are:

- every command highlights only related named parameters -- e.g. 
`add_library`
  do not have `TARGET` named option, so it doesn't highlight in this 
context;
- new highlight classes have introduced to distinct property names, aliased
  (imported) targets, special non-named arguments;
- reuse RST syntax for comments;
- the input data now in VCS and trackable.

REVISION DETAIL
  https://phabricator.kde.org/D7175

To: turbov, dhaumann, #kate, #framework_syntax_highlighting, vkrause, cullmann
Cc: cullmann, #frameworks, michaelh, ngraham, bruns

D7175: Redesign CMake syntax

[Christoph Cullmann]

cullmann accepted this revision.
cullmann added a comment.


  Yes, please push your change.
  Thanks for the work!

REVISION DETAIL
  https://phabricator.kde.org/D7175

To: turbov, dhaumann, #kate, #framework_syntax_highlighting, vkrause, cullmann
Cc: cullmann, #frameworks, michaelh, ngraham, bruns

D7175: Redesign CMake syntax

[Dominik Haumann]

dhaumann added a comment.


  Oh, and btw. sorry that this review took essentially 7 months. Sometimes we 
simply suck and things go badly wrong :-)

REVISION DETAIL
  https://phabricator.kde.org/D7175

To: turbov, dhaumann, #kate, #framework_syntax_highlighting, vkrause
Cc: cullmann, #frameworks, michaelh

D7175: Redesign CMake syntax

[Dominik Haumann]

dhaumann accepted this revision.
dhaumann added a comment.
This revision is now accepted and ready to land.


  So everyone seems to agree that in general it's a good idea.
  
  @turbov Please go ahead and commit. Don't forget to run make test, and 
possibly update the unit test rererence with 
autotests/update-reference-data.sh. Maybe you even want to extend the 
autotest/input/*.cmake files. Please maintain it also for the future :-)

REVISION DETAIL
  https://phabricator.kde.org/D7175

To: turbov, dhaumann, #kate, #framework_syntax_highlighting, vkrause
Cc: cullmann, #frameworks, michaelh

D7175: Redesign CMake syntax

[Alex Turbov]

turbov updated this revision to Diff 26933.
turbov added a comment.


  Updates for CMake 3.11

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7175?vs=23100=26933

REVISION DETAIL
  https://phabricator.kde.org/D7175

AFFECTED FILES
  data/generators/cmake-gen.sh
  data/generators/cmake.xml.tpl
  data/generators/cmake.yaml
  data/generators/generate-cmake-syntax.py
  data/syntax/cmake.xml

To: turbov, dhaumann, #kate, #framework_syntax_highlighting, vkrause
Cc: cullmann, #frameworks, michaelh

D7175: Redesign CMake syntax

[Alex Turbov]

turbov added a comment.


  In https://phabricator.kde.org/D7175#202300, @vkrause wrote:
  
  > I'm with Christoph on this. Having the generated code checked in is fine, 
I'd like to avoid running this on compile time due to the extra dependencies.
  
  
  It doesn' needed to run anything @ compile time.
  
  Just to clarify things once again:
  
  - The generator use only YAML as input
  - The YAML file is manually maintained! (cuz grabbing output of `cmake 
--help...` doesn't work well for a long time already)
  - I do updates of YAML w/ every CMake release after reading release notes 
line by line (cuz I do a lot of CMake code in may daily job and use the latest 
version all the time)
  
  If that is OK 4 u guys, I'll commit...

REVISION DETAIL
  https://phabricator.kde.org/D7175

To: turbov, dhaumann, #kate, #framework_syntax_highlighting, vkrause
Cc: cullmann, #frameworks, michaelh, ngraham

D7175: Redesign CMake syntax

[Christoph Cullmann]

cullmann added a comment.


  If the new highlighting better covers what CMake has around on features now 
than the old, I am for adding the new one.
  
  If the generator is optimal or not is an other thing, but as long as we would 
just check in the result and have the generator as addition to be able to rerun 
on demand, I think that is no issue.
  (if the file is handcraftet updates are much harder)
  
  But Volker may decide as maintainer of the framework.

REVISION DETAIL
  https://phabricator.kde.org/D7175

To: turbov, dhaumann, #kate, #framework_syntax_highlighting, vkrause
Cc: cullmann, #frameworks, michaelh, ngraham

D7175: Redesign CMake syntax

[Alex Turbov]

turbov added a comment.


  @cullmann , @dhaumann
  
  if there is no intereset, I'll close this review and will distribute my CMake 
syntax in other ways...

REVISION DETAIL
  https://phabricator.kde.org/D7175

To: turbov, dhaumann, #kate, #framework_syntax_highlighting, vkrause
Cc: cullmann, #frameworks, michaelh, ngraham

D7175: Redesign CMake syntax

[Alex Turbov]

turbov added a comment.


  ping

REVISION DETAIL
  https://phabricator.kde.org/D7175

To: turbov, dhaumann, #kate, #framework_syntax_highlighting, vkrause
Cc: cullmann, #frameworks

D7175: Redesign CMake syntax

[Alex Turbov]

turbov updated this revision to Diff 23100.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7175?vs=6=23100

REVISION DETAIL
  https://phabricator.kde.org/D7175

AFFECTED FILES
  data/generators/cmake-gen.sh
  data/generators/cmake.xml.tpl
  data/generators/cmake.yaml
  data/generators/generate-cmake-syntax.py
  data/syntax/cmake.xml

To: turbov, dhaumann, #kate, #framework_syntax_highlighting, vkrause
Cc: cullmann, #frameworks

D7175: Redesign CMake syntax

[Alex Turbov]

turbov updated this revision to Diff 6.
turbov edited the summary of this revision.
turbov added a comment.


  Updated for CMake 3.10

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7175?vs=21208=6

REVISION DETAIL
  https://phabricator.kde.org/D7175

AFFECTED FILES
  data/generators/cmake-gen.sh
  data/generators/cmake.xml.tpl
  data/generators/cmake.yaml
  data/generators/generate-cmake-syntax.py
  data/syntax/cmake.xml

To: turbov, dhaumann, #kate, #framework_syntax_highlighting, vkrause
Cc: cullmann, #frameworks

D7175: Redesign CMake syntax

[Alex Turbov]

turbov updated this revision to Diff 21208.
turbov added a comment.


  Include updates for CMake 3.10

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7175?vs=21207=21208

REVISION DETAIL
  https://phabricator.kde.org/D7175

AFFECTED FILES
  data/generators/cmake-gen.sh
  data/generators/cmake.xml.tpl
  data/generators/cmake.yaml
  data/generators/generate-cmake-syntax.py
  data/syntax/cmake.xml

To: turbov, dhaumann, #kate, #framework_syntax_highlighting, vkrause
Cc: cullmann, #frameworks

D7175: Redesign CMake syntax

[Alex Turbov]

turbov updated this revision to Diff 21207.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7175?vs=18322=21207

REVISION DETAIL
  https://phabricator.kde.org/D7175

AFFECTED FILES
  data/generators/cmake.yaml
  data/generators/generate-cmake-syntax.py
  data/syntax/cmake.xml

To: turbov, dhaumann, #kate, #framework_syntax_highlighting, vkrause
Cc: cullmann, #frameworks

D7175: Redesign CMake syntax

[Alex Turbov]

turbov added a comment.


  In https://phabricator.kde.org/D7175#138554, @dhaumann wrote:
  
  > What is still unclear to me: As I understand we from time to time run the 
generator script (point 2 in my list). So this is not automatically run when 
executing "make", right?
  
  
  correct. it doesn't run on `make` and never was. Some time ago I've imroved 
that generator and know its limitations and pitfals -- yeah, it is too simple 
to "parse" informal (in fact) output of `cmake --help-xxx` commands correctly.
  That is the main goal I pursue doing that redesign -- to make sure that all 
commands and parameters are here (in keyword lists) is not a mistake (error in 
the current generator).
  And most important, at least to me, to highlight only really existed named 
keywords for particular command, giving a hint to a user that particular 
parameter is really exists in this particular command.
  
  > I would be fine with this change, but the other point still stand: remove 
hard-coded colors :-) And do we have a test case for this? If not, please add a 
test file.
  
  yeah, I'll do that as soon as realize that this MR have a chance to be 
accepted... %)
  
  > Also, since you redesign this file, I think it would be OK to relicense to 
MIT.
  
  sure

REVISION DETAIL
  https://phabricator.kde.org/D7175

To: turbov, dhaumann, #kate, #framework_syntax_hightlighting, vkrause
Cc: cullmann, #frameworks

D7175: Redesign CMake syntax

[Dominik Haumann]

dhaumann added a comment.


  What is still unclear to me: As I understand we from time to time run the 
generator script (point 2 in my list). So this is not automatically run when 
executing "make", right?
  
  I would be fine with this change, but the other point still stand: remove 
hard-coded colors :-)
  Also, since you redesign this file, I think it would be OK to relicense to 
MIT.
  
  @vkrause What's your take on this?

REVISION DETAIL
  https://phabricator.kde.org/D7175

To: turbov, dhaumann, #kate, #framework_syntax_hightlighting, vkrause
Cc: cullmann, #frameworks

D7175: Redesign CMake syntax

[Alex Turbov]

turbov updated this revision to Diff 18322.
turbov added a comment.


  Remove spaces around ``

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7175?vs=17801=18322

REVISION DETAIL
  https://phabricator.kde.org/D7175

AFFECTED FILES
  data/generators/cmake-gen.sh
  data/generators/cmake.xml.tpl
  data/generators/cmake.yaml
  data/generators/generate-cmake-syntax.py
  data/syntax/cmake.xml

To: turbov, dhaumann, #kate, #framework_syntax_hightlighting, vkrause
Cc: cullmann, #frameworks

D7175: Redesign CMake syntax

[Alex Turbov]

turbov added a comment.


  In https://phabricator.kde.org/D7175#133152, @dhaumann wrote:
  
  > I see seveal issues that imo should be addressed:
  >
  > 1. Please add a test case in autotest/input/ or extend it if one already 
exists (did you run make test?)
  
  
  no, I don't run tests yet (it is too complicated to build the repo for me 
nowadays)
  
  > 
  > 
  > 2. How does it work, are we supposed to run this command from time to time 
manually? If so, then I'd be ok with that.
  
  it work pretty simple:
  
  1. reading YAML (as a Python's `dict`)
  2. prepare (rename keys, & etc) data for renderer
  3. render data (a `dict`) into the Jinja template
  
  as I said the previous generator became unstable -- parsing the output of 
`cmake --help` is not reliable, so this generator uses "static" YAML file, 
which should be updated manually
  
  > 
  > 
  > 3. How high is the maintenance burden over time: I understand that this 
should this, but given we have a yaml file that needs to be maintained 
manually, I wonder whether we simply move the maintenance from one place to 
another and additionally introduce complexity (in terms of additional tooling) 
one first needs to understand before being able to fix things.
  
  the point w/ the current generator: I'm far from sure it "parse" everything 
correct... moreover, I know exactly that it "parses" `cmake --help` output 
incorrectly (and one have to fix the generated syntax manually after the 
generator) and there is really hard to make the generator right cuz the 
informal/irregular nature of help screens.
  in contrast, the new approach offers a full control over the generated 
syntax, yeah, by the cost of "manual" maintenance of the YAML file (I tried to 
make it as simple as possible, and I'm sure it shouldn't be a problem to anyone 
to add anything to it %)
  Usually, I read CMake's ChangeLog carefully after any new release, spotting 
new commands/variables/properties/etc and this is the time to update syntax 
(YAML file)...
  
  > 
  > 
  > 4. The kateversion is back to 2.4 and hard-coded colors are used again. 
Previously, it was set to 5.0, and used already the newly introduced default 
styles. I strongly suggest to keep the new default styles - we purposefully 
changed this some time ago, and I dislike that fact that we go a step back here.
  
  previous cmake.xml do not use anything from modern kate, this syntax works 
fine w/ KDE4 version of kate. 
  anyway, it would be easy to fix it and remove hardcoded colors anyway...
  
  the hardest part is to realize would this approach be accepted or not

REPOSITORY
  R216 Syntax Highlighting

REVISION DETAIL
  https://phabricator.kde.org/D7175

To: turbov, dhaumann, #kate, #framework_syntax_hightlighting, vkrause
Cc: cullmann, #frameworks

D7175: Redesign CMake syntax

[Dominik Haumann]

dhaumann added a comment.


  I see two issues that imo should be addressed:
  
  1. Please add a test case in autotest/input/ or extend it if one already 
exists (did you run make test?)
  
  2. How does it work, are we supposed to run this command from time to time 
manually? If so, then I'd be ok with that.
  
  3. How high is the maintenance burden over time: I understand that this 
should this, but given we have a yaml file that needs to be maintained 
manually, I wonder whether we simply move the maintenance from one place to 
another and additionally introduce complexity (in terms of additional tooling) 
one first needs to understand before being able to fix things.
  
  4. The kateversion is back to 2.4 and hard-coded colors are used again. 
Previously, it was set to 5.0, and used already the newly introduced default 
styles. I strongly suggest to keep the new default styles - we purposefully 
changed this some time ago, and I dislike that fact that we go a step back here.

REPOSITORY
  R216 Syntax Highlighting

REVISION DETAIL
  https://phabricator.kde.org/D7175

To: turbov, dhaumann, #kate, #framework_syntax_hightlighting, vkrause
Cc: cullmann, #frameworks

D7175: Redesign CMake syntax

[Christoph Cullmann]

cullmann added a reviewer: vkrause.
cullmann added a comment.


  If that makes it easier to keep the syntax file up-to-date, I would have no 
issues with a generator build in this way.
  If we ever want to go away from it, we still have the generated file around 
as base.
  But Volker should give his input, too.

REPOSITORY
  R216 Syntax Highlighting

REVISION DETAIL
  https://phabricator.kde.org/D7175

To: turbov, dhaumann, #kate, #framework_syntax_hightlighting, vkrause
Cc: cullmann, #frameworks

D7175: Redesign CMake syntax

[Alex Turbov]

turbov created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  CMake syntax has been redesigned to pursue the following goals:
  
  - the former generator has used an output of `cmake --help` (and friends) 
commands, which is quite informal (not a well formed) and error prune
  - the current syntax was really out of date
  
  The new approach is to use a trivial Python script, which uses a prepared 
YAML data file and Jinja template to render the resulting syntax file.
  Key features of the new implementation are:
  
  - every command highlight only related named parameters -- e.g. `add_library` 
do not have `TARGET` named option, so it doesn't highlighted in this context
  - new highlight classes has introduced to distinct property names, aliased 
(imported) targets, special non-named arguments...
  - reuse RST syntax for comments
  - input data now in VCS and trackable
  
  there is still some work to do, but I've just want to know is there any 
interest in this approach...

REPOSITORY
  R216 Syntax Highlighting

REVISION DETAIL
  https://phabricator.kde.org/D7175

AFFECTED FILES
  data/generators/cmake-gen.sh
  data/generators/cmake.xml.tpl
  data/generators/cmake.yaml
  data/generators/generate-cmake-syntax.py
  data/syntax/cmake.xml

To: turbov
Cc: #frameworks