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

refactor: naming conventions and styling #121

Merged
merged 3 commits into from
May 15, 2016

Conversation

ldez
Copy link
Member

@ldez ldez commented May 13, 2016

Description

  • htmlentities become character-reference
  • reference become xref
  • code-block respect conventions
  • better code blocks patterns

Syntax example

[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

@ldez ldez added this to the v1.3.0 milestone May 13, 2016
@ldez ldez force-pushed the refactor/naming-conventions-styling branch 4 times, most recently from dc2162e to abdb59c Compare May 14, 2016 14:50
@ldez ldez added the ready label May 14, 2016
@ldez ldez force-pushed the refactor/naming-conventions-styling branch from abdb59c to 631f2dc Compare May 14, 2016 22:24
- `htmlentities` become `character-reference`
- `reference` become `xref`
- ordering `explicit` after `code-block`
- `code-block`  respect conventions
-  better code blocks patterns
@ldez ldez force-pushed the refactor/naming-conventions-styling branch from 631f2dc to 5e202ea Compare May 14, 2016 22:41
@mojavelinux
Copy link
Member

👍 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
Copy link
Member

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
Copy link
Member Author

ldez commented May 15, 2016

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
Copy link
Member Author

ldez commented May 15, 2016

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

@ldez
Copy link
Member Author

ldez commented May 15, 2016

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
Copy link
Member

mojavelinux commented May 15, 2016

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

@mojavelinux
Copy link
Member

mojavelinux commented May 15, 2016

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
Copy link
Member

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

@mojavelinux
Copy link
Member

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
Copy link
Member Author

ldez commented May 15, 2016

@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.

@ldez ldez force-pushed the refactor/naming-conventions-styling branch from 449c70c to eb261b8 Compare May 15, 2016 12:03
@ldez ldez force-pushed the refactor/naming-conventions-styling branch from eb261b8 to 37e1597 Compare May 15, 2016 12:07
@ldez ldez merged commit f9c37d0 into asciidoctor:master May 15, 2016
@ldez ldez deleted the refactor/naming-conventions-styling branch May 15, 2016 12:10
@nicorikken
Copy link
Contributor

@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. 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants