[Question] 'support.type.class' scope design decision

[abusalimov]

Hi,

I'm a developer of the Sublime C Improved package and I'm thinking about to port it also to Atom and VS Code. Recently I was asked to add an ad-hoc highlighting of TODO, FIXME, etc. task tags within the syntax definition, since ST doesn't support injectionSelector, which powers the language-todo package and its TM ancestor. I aim to keep the syntax compatible with existing solutions as much as possible, and that's how I came across this package.

However, I'm confused about the support.type.class choice for the task tags. The thing is, I'd like to make a PR for a color scheme customizing highlighting of TODO, FIXME, etc. task tags, and I'm stuck for specifying the right scope selector for that.
Obviously I can't just match support.type.class because it also covers valid source tokens like class or struct.
Should it be (comment | text.plain) support.type.class then? And what if the list of injection scopes gets expanded like proposed in #36?

Basically, what is the rationale behind choosing this particular scope for task tags? Am I missing something or it is likely just a legacy scope retained for backward compatibility? If so, what's about attaching additional more specific scopes for task tags?

[abusalimov]

Here's what I mean by additional scopes.

1. One option could be just adding two distinct scopes at once (storage.other.task-tag):

        'match': '(?<!\\w)@?(TODO|FIXME|CHANGED|XXX|IDEA|HACK|NOTE|REVIEW|NB)\\b'
   -    'name': 'storage.type.class.${1:/downcase}'
   +    'name': 'entity.other.task-tag.${1:/downcase} storage.type.class.${1:/downcase}'
      }

2. Another way is to also match the whole line containing a task tag as a meta.toc-list.task-tag scope, roughly like:

   'patterns': [
   {
     'begin': '[\\s=/*+\\-]*(?=.*?((?<!\\w)@?(TODO|FIXME|CHANGED|XXX|IDEA|HACK|NOTE|REVIEW|NB)\\b|<(ra?dar:/(?:/problems?|)/(?:[&0-9]+))>))'
     'end': '(?=[\\s=/*+\\-]*\\*/)|(?<=$\\n)'
     'contentName': 'meta.toc-list.task-tag'
     'patterns': [
       {
         'match': '(?<!\\w)@?(TODO|FIXME|CHANGED|XXX|IDEA|HACK|NOTE|REVIEW|NB)\\b'
         'name': 'storage.type.class.${1:/downcase}'
       }
       {
         'captures':
           '1':
             'name': 'markup.underline.link.radar'
         'match': '<(ra?dar:/(?:/problems?|)/(?:[&0-9]+))>'
         'name': 'storage.type.class.radar'
       }
     ]
   }
   ]

In this way, a comment containing a task tag would have the following structure:

// TODO: Highlight this line.
   ^^^^-------------------------------------- 'storage.type.class.todo' (for backward compatibility)
   ^^^^---------------------------------- 'entity.other.task-tag.todo' (for more precise and simple selectors)
   ^^^^^^^^^^^^^^^^^^^^^^^^^^-------- 'meta.toc-list.task-tag'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^---- 'comment.line.double-slash'

Also users could choose to style whole lines as well.

Here's an example using C Improved:

Please let me know what you think, and whether you are interested in such changes or not.

[abusalimov]

Choosing the right scope is an open issue though.

On the one hand, it could be entity.other..., I guess:

entity — an entity refers to a larger part of the document, for example a chapter, class, function, or tag. We do not scope the entire entity as entity.* (we use meta.* for that). But we do use entity.* for the “placeholders” in the larger entity, e.g. if the entity is a chapter, we would use entity.name.section for the chapter title.

On the other hand, a task tag is more like a keyword. But:

keyword — keywords (when these do not fall into the other groups)

And that other group could really be storage.other..., but still not storage.type.class... I guess.

The proper order of the old and new scopes is also an open question:

• xxx.other.task-tag storage.type.class (new old)
  • Pros: Fully backward compatible, no impact on users of existing color schemes
  • Cons: Color scheme authors and users, who want to customize task tags highlighting in a generic way, need to specify both scopes (otherwise storage.type takes precedence). Implies that the old support.type.class scope is retained forever for compatibility
• storage.type.class xxx.other.task-tag (old new)
  • Pros: Customizing the highlighting in a generic way is simplified since the new scope takes precedence
  • Cons: Depending on the new scope (entity/keyword/storage), existing color schemes may change the highlighting. Also I'm not sure how user style sheets (like suggested in Different Colors/Patterns [Question] #19) would behave in this case.

One more thing that I think could be addressed more or less easily is classifying task tags into categories by priority:

• prio-high: FIXME, XXX, WTF, BUG, ...
• prio-normal: TODO
• prio-low: TBD, REVIEW, HACK
• other: NOTE, NB, CHANGED, IDEA, IMPORTANT

This would let color schemes style task tags in a generic way (e.g. red/yellow/green highlighting like shown on the screenshot above).

[abusalimov]

Sorry for that long mindflow. Let me summarize it as a survey:

• Introducing new scopes as aliases for the existing ones (to be able to use a single generic selector reliably):
  • Sounds good / Neutral, would accept a PR
  • Against this
• Task tags categories / priorities (extending/specializing generic scopes):
  • Sounds good / Neutral, would accept a PR
  • Against this
• New meta scope for the whole line (can be highlighted differently, also indexed as table of content in Ctrl+R symbol list):
  • Sounds good / Neutral, would accept a PR
  • Against this / The implementation is clumsy (it really is)
• Proper scope:
  • entity.other.task-tag.prio-{normal,high,low}.{todo,fixme,note,...}
  • keyword.other.task-tag...
  • storage.other.task-tag...
• Scope precedence:
  • new, then old (the old one takes precedence, full backward compatibility)
  • old, then new (the new one overrides the old)

Please feel free to check the marks above (contributors have editing rights). Comments are appreciated.

[Alhadis]

Having developed ~15 language grammars myself, I long ago came to the conclusion that semantics are secondary to clarity of highlighting. support.type.class is a well-supported scope-name that's likely to be prioritised by most, if not all, syntax themes. As such, it has a higher chance of standing out than, say, other.meta.task-keyword or what-have-you.

In short, the semantics of scope-choices aren't applicable to all grammar contexts, especially injections.