Skip to content
This repository has been archived by the owner on Dec 28, 2022. It is now read-only.

Code block for properties continue parsing after the end of the code block #59

Closed
ldez opened this issue Apr 23, 2016 · 10 comments
Closed

Comments

@ldez
Copy link
Member

ldez commented Apr 23, 2016

Currently in the (next version of the) plugin, properties languages is related to source.git-config.

But language-git plugin parsing have a side effect:

  • in git config.cson
  • the repository.section grammar have end marker 'end': '(?=\\[)', this means block (section) ending is the start of an another section

2 choices are possible:

  • change the language-git plugin grammar
  • use source.java-properties instead of source.git-config. But source.java-properties don't support properly git-config syntax (fail with [section] block).

@nicorikken wdyt ?

@nicorikken
Copy link
Contributor

I guess I'm missing the point. I'll have to look at some source code, or maybe you can provide a minimal example. Either I would like to see a fix in asciidoctor-language itself, to avoid divering from upstream for referenced content. I consider the Java fix also to be a form of divergent, so given the 2 options, I'd say neither. I'll need to look into this further.

@ldez
Copy link
Member Author

ldez commented Apr 23, 2016

properties language is commonly java-properties but in GitHub properties is related to a git-config like syntax.

There is no others choices: the grammar parser think the code block is not closed and it will parse the rest of the file. Change regular expression in language-asciidoc cannot change this behavior.

See: https://github.com/asciidoctor/atom-language-asciidoc/blob/master/spec/fixtures/asciidoctor-lang.adoc#properties-bug-

@mojavelinux
Copy link
Member

This is one of the things I find very limiting about the Atom/Textmate grammar system. You can set beginCaptures and endCaptures, but it seems like you can't make the inner-match non-greedy. In other words, the endCaptures seems to capture the last instance. Is my understanding correct?

Surely there must be a way to limit the scope of the content being passed to the nested scope. If that's possible, then there should be no way for the git-config matcher should be allow to extend outside of the block.

@ldez
Copy link
Member Author

ldez commented Apr 23, 2016

If you capture a block with nested grammar, you must wait the end of the nested grammar before defined the end of the block.

It's very limited.

Before open this issue I've try a lot of things but all fails...

The only solution without change language-git or switch to java-properties is:

# comment
 [user]
    name = John Doe
    email = example@examplecom
[]

add a [] as a last line but it's very very ugly.

@mojavelinux
Copy link
Member

That is unfortunate. I wonder if we can start working upstream in Atom to get more control over where the nested grammar wins. It may not help us with a short-term solution here, but it could pave the way for more robust behavior in the future (and not just for this block).

Thank you for taking the time to dig into this. I agree that the hack you discovered is ugly. I'd be open to either a) not supporting the language-git or b) handling the parsing ourselves until something better comes along upstream.

In other words, I'm proposing we work in phases to get this solved.

@ldez
Copy link
Member Author

ldez commented Apr 24, 2016

I have created a "custom properties" grammar embedded in the plugin,.
This solve the problem but it's not very DRY.

I waiting the merge of #58 before create a PR or I can add this to the #58 ?
wdyt ?

@mojavelinux
Copy link
Member

I think we can live without DRY for now. Just add a note about why it is there and next steps so that someone coming along to help understands what's going on.

I think you can add it to #58 since it is tightly related.

ldez added a commit to ldez/atom-language-asciidoc that referenced this issue Apr 24, 2016
@ldez
Copy link
Member Author

ldez commented Apr 24, 2016

Fix in #58

@ldez ldez closed this as completed Apr 24, 2016
@ldez ldez mentioned this issue Apr 24, 2016
5 tasks
ldez added a commit to ldez/atom-language-asciidoc that referenced this issue Apr 24, 2016
ldez added a commit to ldez/atom-language-asciidoc that referenced this issue Apr 24, 2016
@nicorikken
Copy link
Contributor

I falsely assumed an end-marker would be identified based on the top level, preventing the inner patterns to be too greedy (as they were bounded by the previously set end-marker). Too bad. Then I agree with code duplication, although an upstream fix would be better still. @ldez very nice work!

ldez added a commit to ldez/atom-language-asciidoc that referenced this issue Apr 25, 2016
ldez added a commit to ldez/atom-language-asciidoc that referenced this issue Apr 25, 2016
@mojavelinux
Copy link
Member

I think the upstream project is first-mate. Would either of you be able to explain the change we are looking for?

Possibly related:

nicorikken pushed a commit that referenced this issue Apr 27, 2016
* refactor: code block scope

- all the code blocks can be immediately followed by other grammar.
- fix bug with CSS  code block

Fix #20

* refactor(code block): remove space at the beginning of the line.

Fix #39

* feat: add TypeScript language support

* refactor: split SCSS to SCSS and SASS

* feat: add custom properties grammar

Fix #59

* refactor: enhance grammar cson readability
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants