Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

syntax: add reference syntax files for testing lexers #2916

Merged
merged 2 commits into from
Feb 6, 2019

Conversation

oliver-sanders
Copy link
Member

Addresses #2915

Add some example Cylc files for testing lexers, the numbering matches the table in #2752.

Copy link
Collaborator

@sadielbartholomew sadielbartholomew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initial feedback from a first read & cross-reference with the files displayed in emacs.

As a general comment, do we intend to define a standard in terms of what features should display in the same mode (as opposed to what features should each display as single consistent units by mode)? For an example to make that distinction clearer, here we seem to be implying [a], [[d]] & [[l, m, n]] should each individually display in one colour, but should we outline that they should all have the same colour (say to group them as a set of section headings)? Full consistency is not essential but it could be useful for comprehension purposes.

If we wanted to push for consistency in this way, we should provide strict instructions & remove preference advice e.g. optionally <PATTERN> could be highlighted instances.

etc/syntax/reference-files/suite.rc Outdated Show resolved Hide resolved
etc/syntax/reference-files/suite.rc Show resolved Hide resolved
etc/syntax/reference-files/suite.rc Outdated Show resolved Hide resolved
etc/syntax/reference-files/suite.rc Outdated Show resolved Hide resolved
etc/syntax/reference-files/suite_empy.rc Outdated Show resolved Hide resolved
etc/syntax/reference-files/suite_empy.rc Outdated Show resolved Hide resolved
etc/syntax/reference-files/suite_jinja2.rc Outdated Show resolved Hide resolved
{% from 'foo' import foo as a, b %}

# SYNTAX 3.3
# multi-line templating code should be consistent with single-line code
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps to convey that all four examples below are meant to display consistently as a unit, though only the top two are commented as such at the moment, add instead one line at the top here similar to # for each case, all three lines should display as a single unit.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kinda depends how template markup is handled, for instance the syntax highlighter might wish to highlight keywords or literals differently.

I've updated the comment to make extra clear that whatever the behaviour implemented by the lexer it should be consistent no matter how many lines they are spread over.

etc/syntax/reference-files/suite_jinja2.rc Show resolved Hide resolved
@oliver-sanders oliver-sanders force-pushed the cylc-reference-syntax-files branch from 11b399e to 8ca36e3 Compare January 16, 2019 14:40
Copy link
Collaborator

@sadielbartholomew sadielbartholomew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous feedback addressed really well, with the 'SYNTAX: 2.10' now especially comprehensive. I've noticed that the numbering is not consistent between the EmPy file & the master table, & possibly one typo, but otherwise I am happy to approve.

I'll now go & double-check that the revised reference table is correct for Emacs, given my recent update to that lexer.

etc/syntax/reference-files/suite_empy.rc Outdated Show resolved Hide resolved
etc/syntax/reference-files/suite_empy.rc Outdated Show resolved Hide resolved
etc/syntax/reference-files/suite_empy.rc Outdated Show resolved Hide resolved
"""

# syntax 3./2.5
# clashing xtrigger and empy syntax should somehow be handled
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well anticipated!

In general, implementing lexers supporting all the listed syntactical elements cleanly & consistently will be rather tricky I think, for reasons such as this i.e. given different interpretations under the suite.rc mode & the two templating engines.

@oliver-sanders oliver-sanders force-pushed the cylc-reference-syntax-files branch from 8ca36e3 to b4ed8ed Compare January 17, 2019 09:53
@oliver-sanders oliver-sanders modified the milestones: soon, next-release Jan 28, 2019
@kinow
Copy link
Member

kinow commented Feb 6, 2019

Looks good to me. I think these files provide good reference for further work, at least on the vim syntax. e.g.

image

I think %include bar should have different colours for the left and right parts (as in the other lines).

Merging. And sorry for the delay reviewing this one!

@kinow kinow merged commit 3549c0f into cylc:master Feb 6, 2019
@oliver-sanders
Copy link
Member Author

I think %include bar should have different colours for the left and right parts

Yep, that would be nice!

@oliver-sanders oliver-sanders deleted the cylc-reference-syntax-files branch February 7, 2019 11:46
@matthewrmshin matthewrmshin mentioned this pull request Feb 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants