sublimehq / Packages

Syntax highlighting files shipped with Sublime Text and Sublime Merge
https://sublimetext.com
Other
2.95k stars 586 forks source link

[Go] Add back builtin type matching #1803

Open kortschak opened 5 years ago

kortschak commented 5 years ago

I know that this goes against some of the tauted benefits of #1662 in that there is now no longer a need for a 'whitelist of "known" types'.

I use a semantic syntax highlighting, and previously had to use my own syntax definition forked from the old syntax file shipped with GoSublime. The new syntax definition gives me nearly all of what I have with my hand rolled (and not as clean) syntax file, except that I would like to diminish builtin types, so without a whitelist of known types, I cannot do this.

Would it be possible to add back the builtin types? Alternatively some way of importing existing definitions and extending them would work (I'd like unsafe and unsafe.Pointer to be given a scope as well and I don't expect that this would be considered, so a way of extending existing defs might be better).

mitranim commented 5 years ago

This seems spiritually related to #1795, to which I just posted a baffled response.

It also seems that many incumbents prefer scoping built-ins, even to the exclusion of user-defineds. I personally don't understand this, hence the new syntax definition with the general-case type support.

Would you mind explaining your ideal experience to bring us on the same page? For example, I'm curious what you mean by "diminishing built-in types" and "scoping unsafe and unsafe.Pointer".

Feedback on the general-case type support would also be useful.

Unlike most languages, Go makes it easy to syntactically differentiate "local" and "builtin" identifiers from "public" and "external", which is one of the reasons I think scoping the built-ins is unnecessary. That said, collaborative projects like these tend to err on the side of "more optional features" rather than "one true way of doing things", so with enough nagging, we're likely to restore the feature. More discussion would be nice though.

kortschak commented 5 years ago

Thanks for the detailed query.

It's not very related to that issue, though I can see why you see a similarity.

As I mentioned in the issue, I use hashed color scheme (semantic colouring) for labels in Go code. I use a different colour range and italicisation for builtins than for user-defined types. I also have a prominant highlighting for unsafe.Pointer (picked up while working with a large unsafe code translation.

I have to say that the way the syntax definitions work now is very nice apart from these minor points (the color-scheme file is much clearer with the new defs than it was with my hacked together one).

Happy to clarify further if this doesn't answer your questions.

mitranim commented 5 years ago

Hmm. Even though I still don't understand the intent behind "styling built-in types differently from user-defined types", I don't have weighty arguments against it. What really weirds me out is when folks don't seem to care at all about scoping "user-defined" stuff, or haven't given it time to sink in.

You mentioned label scoping. Are you referring specifically to block labels, or identifiers in general? The "new" syntax doesn't support block labels yet. I plan to add this feature, but it also requires us to detect data structure literals so we can differentiate field names from labels; this is next on my roadmap.

I agree that the ability to extend a syntax without duplicating the definition would be ideal for what we're discussing. I maintain local modifications to some syntaxes, such as embedding regexp or PostgreSQL syntax in Go strings, and it's currently rather fiddly. There are open issues and PRs of a similar character, which are unlikely to ever be accepted into the core.

The unsafe package is interesting. It thwarts my usual argument that standard library packages are entirely user-defined as far as the language is concerned. unsafe is a facade, a clever way to avoid introducing special syntax or keywords for built-in compiler features. If we decide to support built-ins, there's a solid argument that unsafe is part of that. A solid counter argument is that we should stick to what's in the language spec. I hear that some Go environments, such as GopherJS, Google App Engine and wasm, either don't support unsafe or fake it out. We did make an exception for annotation comments, but maybe that's going too far. Again, ideally this kind of feature would be reserved for dynamic syntax extensions. Also, is there a widely-used scope that would make logical sense for unsafe stuff? It pretends to be just types and functions, after all.

kortschak commented 5 years ago

I'm using the term label to talk about identifiers. Here's an example (the lurid colours work for me, though they take a little getting used to).

screenshot from 2018-12-08 22-25-17

The absence of support for unsafe I think is actually an argument for having it special-cased in the syntax. That was when I found the utility of having this. I was progressively removing unsafe.Pointer use from a few thousand lines of manually translated C. Seeing them stand out was very helpful. Note also that unsafe.Pointer is specified. I called the scope unsafe.go in mine (that only matched \bunsafe\b which is enough of a highlight).

mitranim commented 5 years ago

Haha good catch about unsafe being part of the spec. Totally forgot this bit.

Not sure how we want to handle this, though. Following a recent discussion: [1], [2], we probably want built-in scoping to be additive, keeping the standard storage.type and such. Built-in types might receive additional non-meta scopes, and any unsafe.X identifier might receive a meta scope such as meta.unsafe.go.

What really bothers me about built-in scoping is how it automatically breaks on redefinition. Example:

var int = "text"
fmt.Println(int)

var unsafe = "text"
fmt.Println(unsafe)

This is perfectly legal Go. A naive syntax definition would scope this int as a built-in type, which is completely wrong. One obvious solution would be to limit this to places that are already scoped as types. Then, what about casts?

var _ = int(123)

var int = strconv.Atoi

var _, _ = int("456")

Should we attempt to scope the first "call" as a built-in type? In that case, the second call will be incorrectly scoped as a type rather than a function.

Making the syntax so fragile seems like a disservice. To be fair, the syntax has already crossed the line by special-casing true, false, nil, new and make, which may also be redefined. Special rules like "only scope them when they're types or function calls" also increase the brittleness.

I would lean towards having fewer features, but being reliably correct 100% of the time.

Apologies for the brain stream. Probably need time to think it over.

kortschak commented 5 years ago

I agree with all the above. The thing to consider though is that conventionally, while new, len and cap are sometimes redefined (I've never seen other built in functions redefined - I'd actually argue against the special casing of new, len and cap because they are occasionally redefined, even the stdlib), it's very rare (and I'd say frowned upon) to redefine built in types and nil, true and false (the int, unsafe and strconv.Atoi examples you posted are horrific :slightly_smiling_face:).

mitranim commented 5 years ago

Should be mostly addressed by #1825.

Doesn't include support for unsafe, which seems slightly unrelated and a ridiculously rare edge case. If still relevant, feel free to nag me into it.

kortschak commented 5 years ago

Doesn't include support for unsafe, which seems slightly unrelated and a ridiculously rare edge case. If still relevant, feel free to nag me into it.

Pleeeeaaaaaase.

Think about it this way. People who are working with unsafe on on the edge of what should be done. Do you want your intercontinental pilot to be uncomfortable in the cockpit and having to squint to read the dials, or have instrumentation and controls that make your flight safe?

kortschak commented 5 years ago

Are we there yet?

mitranim commented 5 years ago

Still looking into this and scratching my head. There are difficulties. Waiting for a revelation while working on other issues.