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

Improve inline attribute naming and styling #110

Closed nicorikken closed 8 years ago

nicorikken commented 8 years ago

Direct derivative of PR #94

Current naming scheme (strong example):

  # Matches strong unconstrained phrases
  #
  # Examples
  #
  #   s**trong** phrase
  #
  name: 'markup.strong.unconstrained.asciidoc'
  match: '(?<!\\\\\\\\)(\\[.+?\\])?(\\*\\*)(.+?)(\\*\\*)'
  captures:
    1: name: 'markup.meta.attribute-list.asciidoc'
    2: name: 'punctuation.definition.bold.asciidoc'
    3: name: 'markup.bold.strong.asciidoc'
    4: name: 'punctuation.definition.bold.asciidoc'

Current view state:

screen shot 2016-05-11 at 07 58 44
ldez commented 8 years ago

I changed the matching pattern in #111:

  # Matches strong unconstrained phrases
  #
  # Examples
  #
  #   s**trong** phrase
  #
  name: 'markup.strong.unconstrained.asciidoc'
  match: '(?<!\\\\\\\\)(\\[.+?\\])?((\\*\\*)(.+?)(\\*\\*))'
  captures:
    1: name: 'markup.meta.attribute-list.asciidoc'
    2: name: 'markup.bold.strong.asciidoc'
    3: name: 'punctuation.definition.bold.asciidoc'
    5: name: 'punctuation.definition.bold.asciidoc'
mojavelinux commented 8 years ago

I'll repeat the recommendation I laid out in #94 for reference:

markup.strong.unconstrained.asciidoc - [role]**strong text**
  entity.attribute-list.asciidoc - [role]
  punctuation.definition.uncontrained.asciidoc - **
  (no name assignment) - strong text
  punctuation.definition.uncontrained.asciidoc - **

I think we can rely more an CSS inheritance to keep the names simpler. I repeat the "unconstrained" because it qualifies the "punctuation.definition". Another option is "markup.punctuation.uncontrained". I don't think we should include ".bold" on the punctuation as it is redundant and complicates the styling.

mojavelinux commented 8 years ago

@ldez There's no need to capture the formatted text if we don't need to assign a name to it. So we can remove that group.

mojavelinux commented 8 years ago

Perhaps it's time to think about using named capture groups for clarity.

name: 'markup.strong.unconstrained.asciidoc'
  match: '(?<!\\\\\\\\)(?<attrs>\\[.+?\\])?(?<punct>\\*\\*).+?(?<punct>\\*\\*)'
  captures:
    attrs: name: 'meta.attribute-list.asciidoc'
    punct: name: 'punctuation.definition.asciidoc'
ldez commented 8 years ago

named capture groups in captures atrributes doesn't work.

mojavelinux commented 8 years ago

Damn. I just realized that. Bummer.

ldez commented 8 years ago

It works only as group reference in a regular expression

ldez commented 8 years ago

in:

markup.strong.unconstrained.asciidoc - [role]**strong text**
  entity.attribute-list.asciidoc - [role]
  punctuation.definition.uncontrained.asciidoc - **
  (no name assignment) - strong text
  punctuation.definition.uncontrained.asciidoc - **

entity.attribute-list.asciidoc doesn't work as expected

mojavelinux commented 8 years ago

We just need to move around some CSS.

Here's my latest proposal:

  name: 'markup.strong.bold.unconstrained.asciidoc'
  match: '(?<!\\\\\\\\)(\\[.+?\\])?(\\*\\*).+?(\\*\\*)'
  captures:
    1: name: 'entity.attribute-list.inline.asciidoc'
    2: name: 'punctuation.definition.asciidoc'
    3: name: 'punctuation.definition.asciidoc'

The styles for .entity.attribute-list (or just .attribute-list) need to undo the color, font-weight and font-style set by the .bold parent.

mojavelinux commented 8 years ago

I think we should move "unconstrained" to the punctuation. wdyt?

  name: 'markup.strong.bold.asciidoc'
  match: '(?<!\\\\\\\\)(\\[.+?\\])?(\\*\\*).+?(\\*\\*)'
  captures:
    1: name: 'entity.attribute-list.inline.asciidoc'
    2: name: 'punctuation.definition.unconstrained.asciidoc'
    3: name: 'punctuation.definition.unconstrained.asciidoc'
mojavelinux commented 8 years ago

I could go either way. I can't anticipate which placement is more helpful.

ldez commented 8 years ago

You're trying with current master but it contains bad styling practice. Try with my branch.

mojavelinux commented 8 years ago

Oh, okay.

mojavelinux commented 8 years ago

It works if I use the following rule just under .asciidoc:

    &.inline.attribute-list {
      font-weight: normal;
      font-style: normal;
      color: @syntax-text-color-unobtrusive;
    }

We have to force the font-weight and font-style to normal if we put .bold on the root. The other option is to wrap the formatted text with marker and add .bold to that. I'm happy to do it that way.

ldez commented 8 years ago

It's not a good idea to force font-weigh or font-style to normal

mojavelinux commented 8 years ago

No problem. I'll change the proposal then.

mojavelinux commented 8 years ago

Here's the latest:

  name: 'meta.markup.strong.asciidoc'
  match: '(?<!\\\\\\\\)(\\[.+?\\])?((\\*\\*).+?(\\*\\*))'
  captures:
    1: name: 'entity.attribute-list.inline.asciidoc'
    2: name: 'markup.bold.asciidoc'
    3: name: 'punctuation.definition.unconstrained.asciidoc'
    4: name: 'punctuation.definition.unconstrained.asciidoc'

What I'm not sure about is group 2. Should it include "strong"? I'm not sure.

mojavelinux commented 8 years ago

I also kind of like making the punctuation more translucent.

&.punctuation {
  opacity: 0.5;
}

but that could be bad practice.

mojavelinux commented 8 years ago

I'll be the first to admit that reasoning about these names is not easy :/

ldez commented 8 years ago

as in #111, if you play with custom styling you break some theme.

mojavelinux commented 8 years ago

Sure, but in the case of opacity that's a relative change, so it should work in all themes. But I recognize this is more of a personal style that probably belongs in ~/.atom.

mojavelinux commented 8 years ago

It would be cool if the theme system gave us a way to apply a muted affect to something...but it's not that important.

ldez commented 8 years ago

What is your goal ?

If you see others language package like:

There are no custom stylesheet.

mojavelinux commented 8 years ago

The problem with comparing ourselves to other grammars it two-fold:

  1. AsciiDoc is the most sophisticated markup language, so we can never see the world as simple as Markdown
  2. Most of the stylesheets are focused on programming languages and we just don't fit cleanly in that world

I think we have to have a custom stylesheet to get quality highlighting until the default stylesheets do a better job of addressing our needs.

So our stylesheet becomes a todo list for what the default stylesheets are going to have to address. It is our proposal to them.

Our goal should be to use the naming in the most consistent way so that we have the least amount of custom styles and thus a reasonable proposal for what the default stylesheets should include.

(We went through the same thing with highlight.js. The default just didn't account for the whole state of the world originally. We had to get them to evolve to new constructs).

mojavelinux commented 8 years ago

Given that the Atom team uses AsciiDoc for the Atom user manual, I think we have a strong case for them to include these changes. First, though, we have to decide what we are proposing.

ldez commented 8 years ago

it's on to some element that we are obliged to make customizations like sup or sup but we must not overkilling the styling.

mojavelinux commented 8 years ago

I agree we want to keep it as lean as possible. I'm okay with arranging the names a bit to accomplish that, if necessary.

ldez commented 8 years ago

For me AsciiDoc it's a programming language :wink:

mojavelinux commented 8 years ago

hahaha

ldez commented 8 years ago

Here's the latest:

  name: 'markup.strong.unconstrained.asciidoc'
  match: '(?<!\\\\\\\\)(\\[.+?\\])?((\\*\\*).+?(\\*\\*))'
  captures:
    1: name: 'markup.meta.attribute-list.strong.asciidoc'
    2: name: 'markup.bold.strong.asciidoc'
    3: name: 'punctuation.definition.bold.strong.asciidoc'
    4: name: 'punctuation.definition.bold.strong.asciidoc'

I known it's verbose but it allow easy fine custom styling.

ldez commented 8 years ago

or something like that:

  name: 'markup.strong.unconstrained.asciidoc'
  match: '(?<!\\\\\\\\)(\\[.+?\\])?((\\*\\*)(.+?)(\\*\\*))'
  captures:
    1: name: 'markup.meta.attribute-list.strong.asciidoc'
    2: name: 'markup.bold.strong.asciidoc'
    3: name: 'punctuation.definition.bold.strong.asciidoc'
    4: name: 'text.strong.asciidoc'
    5: name: 'punctuation.definition.bold.strong.asciidoc'
mojavelinux commented 8 years ago

I like the first one better. However, I don't think we need to repeat "strong" on the attribute-list. That doesn't seem to add any value and is already inherited. I'd even argue the same for the punctuation.

I'm still a bit confused why we need "markup" on the attribute-list.

mojavelinux commented 8 years ago

Btw, I've read the difference between entity and meta on http://manual.macromates.com/en/language_grammars.html#naming_conventions a hundred times and I think that page is ambiguous. It wants the entity to be both the whole thing and parts of the thing. However, this is the line that justifies to me that the attribute list should be "meta.attribute-list":

"Sometimes the meta part of the scope will be used only to limit the more general element that is styled"

That seems like a perfect fit for the attribute list. On the other hand, the example "entity.name.function" makes me think it should be "entity.attribute-list".

Given this ambiguity, I'd argue that there isn't a clear choice here and will rely on us to decide.

mojavelinux commented 8 years ago

Of course, "meta" fits the purpose of the attribute list best, as it is metadata. Therefore, my gut tells me that "meta" is the right choice.

nicorikken commented 8 years ago

I like where this is going

ldez commented 8 years ago

Better ?

  name: 'markup.strong.unconstrained.asciidoc'
  match: '(?<!\\\\\\\\)(\\[.+?\\])?((\\*\\*)(.+?)(\\*\\*))'
  captures:
    1: name: 'markup.meta.attribute-list.asciidoc'
    2: name: 'markup.bold.asciidoc'
    3: name: 'punctuation.definition.asciidoc'
    5: name: 'punctuation.definition.asciidoc'