atom / language-go

Go language package for Atom
Other
106 stars 65 forks source link

Treat interface{} and struct{} as builtin types #110

Open db47h opened 7 years ago

db47h commented 7 years ago

Prerequisites

Description

While interface and struct are keywords, the empty interface{} and struct{} are so widely used as type literals in Go code that I believe that they should be also treated as built-in types for syntax highlighting purposes.

EDIT: syntactically, they serve the same purpose.

In code like this:

type Foo struct {
    a int
    b interface{}
    c map[interface{}]string
    d chan struct{}
}

having interface{} and struct{} highlighted the same as int would just make the whole definition look more consistent.

Versions

language-go@0.43.0

Additional information

Here's a short patch that does this (insert just before Terminators):

  {
    'comment': 'Inline empty interface type'
    'match': '(\\binterface)({})'
    'captures':
      '0':
        'name': 'storage.type.interface.go'
      '1':
        'name': 'keyword.interface.go'
      '2':
        'patterns': [
          {
            'include': '#brackets'
          }
        ]
  }
  {
    'comment': 'Inline empty struct type'
    'match': '(\\bstruct)({})'
    'captures':
      '0':
        'name': 'storage.type.struct.go'
      '1':
        'name': 'keyword.struct.go'
      '2':
        'patterns': [
          {
            'include': '#brackets'
          }
        ]
  }

This retains the ability to theme interface and struct as keywords.

cbarrick commented 7 years ago

The keyword struct and the keyword interface are already highlighted the same as other types. Your proposal adds the keyword class to the following braces if they are empty. I am personally against this.

Syntax highlighting serves to differentiate syntactic elements. Keywords and punctuation are different syntactic elements in Go, so punctuation should not be given the keyword class.

However, I looked into alternate solutions to achieve the look you want without muddling the syntax labels, and I discovered that it is impossible to simulate in pure css. But you can probably get this behavior from your init script. Something like:

function colorBraces() {
    var bs = findBraces()
    bs.forEach(b => b.classList.add('keyword'))
    requestAnimationFrame(colorBraces)
}
requestAnimationFrame(colorBraces)
db47h commented 7 years ago

I believe you misunderstood my intent. Right now, the grammar parser properly identifies built-in types (int, int32 and so on) and gives them a class such as storage.type.numeric.go, they are not seen as keywords (and are not keywords but predeclared identifiers according to the Go spec). The fact that some syntax themes have the same highlight for keyword and storage.type may have mislead you.

Granted, interface and struct are keywords. However, interface{} and struct{} are type literals with the same syntactic meaning than int or bool, hence my proposal to highlight them the same way. Of course, I'd leave more complex type literals as-is (i.e. non-empty structs or interfaces).

Also, the patch does not apply the keyword class to the braces, it simply wraps the whole type literal in a storage.type.interface.go class and sets the keyword and punctuation.other.bracket where they belong. As a result, themes could choose how to highlight it, either as storage.type or enforce keyword, and users can ultimately do their own thing in their style.less file.

cbarrick commented 7 years ago

Ah, gotcha. I miscounted the captures (thought 1 was the braces :-!)

willfaught commented 7 years ago

Shouldn't struct and interface be colored the same as other keywords like var and const? They're all listed as keywords in the Go spec.

cbarrick commented 7 years ago

It appears that both struct and interface get tagged as keywords in the editor. I still don't think struct{} and interface{} should be special, because semantically they are not. If anyone has a problem with the current state of affairs, screenshots would be help the discussion.

Otherwise, this should probably be closed.

db47h commented 7 years ago

Well, semantically struct{} is a type literal and struct is a keyword. Can't be any simpler.

Now what I'm really interested in is code legibility. When syntax highlighting turns code into a peacock, one might as well disable it.

As requested, I'll try to demonstrate with a few screenshots with different themes.

Atom-Dark

A good example of that is with the Atom-Dark theme where parsing function definitions can sometimes be hard on the eyes.

Before: language-go_atom-dark

After: language-go_atom-dark-fixed

Note that to get things right with this theme, I have to remove the keyword tag since the theme prioritizes keywords over types.

One-Dark

Things are a less problematic with the One-Dark theme that colorizes keywords and types identically.

Before: language-go_one-dark

After: language-go_one-dark-fixed

Tomorrow Night Saturated

Before: language-go_tomorrow-night-saturated

After: language-go_tomorrow-night-saturated-fixed

Monokai

The ever popular Monokai theme is probably the worst offender when it comes to peacock style.

Before: language-go_monokai

After: language-go_monokai-fixed

Obviously, depending on the user theme and personal threshold where syntax highlighting switches from being helpful to a plain hindrance, this might seem relevant, or not.

db47h commented 7 years ago

I just realized that a function argument of type struct{} is just silly, so just imagine that I wrote interface{} instead. I also failed to mention that I changed the channel types coloring as well.

In a darker environment, the Monokai theme is not that bad. Still, when it comes to reading code, homogeneously colored types make it easier to parse, at least for me.