asciidoctor / atom-language-asciidoc

⚛ AsciiDoc language package for the Atom editor.
https://atom.io/packages/language-asciidoc
MIT License
42 stars 20 forks source link

refactor: naming conventions and styling #121

Closed ldez closed 8 years ago

ldez commented 8 years ago

Description


[source]
----
source 'https://rubygems.org'
gem 'asciidoctor', '{latest-release}'
----

[source,bar]
----
source 'https://rubygems.org'
gem 'asciidoctor', '{latest-release}'
----

[source,subs="attributes+,+replacements,-callouts"]
----
source 'https://rubygems.org'
gem 'asciidoctor', '{latest-release}'
----

[indent=2]
----
    def names
      @name.split ' '
    end
----

[source,java,subs="{markup-in-source}"]
----
System.out.println("Hello *bold* text").
----

[source,xml,subs="attributes+,+replacements,-callouts"]
----
<version>{version}</version>
<copyright>(C) ACME</copyright>

content in 1 element
</1>
----

Screenshots

capture du 2016-05-13 22-46-42

mojavelinux commented 8 years ago

:+1: to the name changes.

However, for the block attributes, I think we need to step back and reconsider how we are matching. I think we should highlight block attribute lines in a generic way and not try to do special things for special values.

For instance, if you write an extension, the following is totally valid:

[foobar]
====
content
====

Case in point, Asciidoctor Diagram introduces a wide range of names to a literal block. See http://asciidoctor.org/docs/asciidoctor-diagram/#creating-a-diagram.

What I think we should do is create a matcher for an attribute list in the form of key1=value1,key2=value2,... (where the values are optional). Then we find the block attribute line (e.g., [blockname,key1=value1,key2=value2]) and match the blockname as a function name and all the other attributes as positional attributes, then key/value pairs).

There is a distinction in AsciiDoc between a "structural container", which is a delimited block, and the context (which is the structural container + blockname). See https://github.com/asciidoctor/asciidoctor.org/issues/223 for details.

mojavelinux commented 8 years ago

We can, of course, match the block attribute lines last so that if you want to do something special for a source block, you still can.

ldez commented 8 years ago

match the block attribute lines last

This make a lot of regression, I've tried and I go back on change.

Currently the fixed list of block attributes allow of avoid lot unwanted highlighting

I propose to you to open an issue for this subject.

ldez commented 8 years ago

I canceled the change related to generic block with attributes only because it created many regressions.

ldez commented 8 years ago

For information, the problem is always the same: we cannot match a specific next or previous line. When we define a patterns within a block with begin and end, we don't limit the capture to this nested pattern. If the nested patterns doesn't match the outer matching doesn't stop.

I works to explore the Oniguruma's specific anchors like \G, \A or \Z. But if I use them it will be in an another PR.

mojavelinux commented 8 years ago

I'll follow-up with another issue and include some additional ideas.

mojavelinux commented 8 years ago

I've opened #126 to discuss this change. It's an important one because otherwise we're matching on a limited set of what AsciiDoc allows.

mojavelinux commented 8 years ago

I moved the second half of that comment to #126.

mojavelinux commented 8 years ago

I works to explore the Oniguruma's specific anchors like \G, \A or \Z.

Those anchors could definitely play a key role. Keep in mind, however, that what I'm encouraging is a separation between the block attribute list and the structural container. The highlighter should not try to assume a relationship except in the cases we've identified as an exception.

ldez commented 8 years ago

@nicorikken I need this PR to be merge to continue.

This PR contains only naming changes now.

I dislike merge without your validation but I'm going to merge.

nicorikken commented 8 years ago

@ldez good you made the call to merged. I'm off in Clojure land now, to wack out a prototype of asciidoctor-clj, a Clojure library to easily integrate AsciidoctorJ. Then my second goals is to use that library in an Asciidoctor Perun plugin. I already did such a plugin previously, but I did all the JRuby stuff myself, and did not use some of the powerful Asciidoctor features. I guess in the coming weeks I'll be less responsive in a similar way. Just catching up this morning ;)

Regarding this issue, good discussion. General matching will be nice, but will have some hurdles. So reducing the scope of this refactor is good. 👍