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

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

Open
1 task done
db47h opened this issue Jan 4, 2017 · 7 comments
Open
1 task done

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

db47h opened this issue Jan 4, 2017 · 7 comments

Comments

@db47h
Copy link
Contributor

db47h commented Jan 4, 2017

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

[email protected]

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

cbarrick commented Jan 4, 2017

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

db47h commented Jan 5, 2017

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

cbarrick commented Jan 6, 2017

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

@willfaught
Copy link

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

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

db47h commented Sep 29, 2017

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

db47h commented Sep 29, 2017

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.

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

No branches or pull requests

3 participants