-
Notifications
You must be signed in to change notification settings - Fork 592
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Go] preliminary support for Go 1.18 #3289
Conversation
|
I have no strong opinion. It has obvious upsides but also some downsides. Search-and-replace opens N files, split-panel becomes slightly more annoying to use.
No reason. Updated to v2. |
FWIW There's no hard restriction with regards to used syntax features and syntax version. V2 just gates some backward incompatible changes to the syntax engine, which require a syntax to opt-in. Most of them are related to how meta scopes are applied in push/set/pop scenarios. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's another set of suggestions.
Appreciate the effort to name contexts. |
Is this also going to support syntax based code folding ? |
Maybe address #3228 as well with this PR? |
Syntax based code folding should work pretty well, once each pushed context receives a meta scope so neiboured parens can be detected as separate folding begin/end markers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The linked issue still contains some open tasks.
Do you plan to integrate them in this PR?
In case those are planned for future PRs, would you mind adding some small reminder issues with existing limitations and possible enhancements to certain code constructs - maybe including small examples?
Apologies for slow responses. The linked issue had multiple progress reports which are all outdated, and it wasn't meant to be a progress status of this PR. However, there are other known defects. I'm building a list now. |
Known defects / missing features:
interface{
Method(Type[TypeArg])
}
func(Type, Type[TypeArg])
interface{
EmbeddedInterface[TypeArg]
}
FuncName[Type[TypeArg]]()
func FuncName(param [][]Type)
namespace.FuncName[TypeArg, TypeArg]()
namespace.FuncName[TypeArg]() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR provides many improvements with regards to type support.
Not supporting block quotes everywhere in fully-qualified identifiers is acceptable. Many other syntaxes don't do so at all, even though valid by language specs.
Work on Java syntax teached me required complexity needed to handle all those typing and generics "perfect" especially with regards to multi-line support.
So I think we could add this PR as improvement and add an issue with the open "TODOs" to come back to them later. Maybe fixing them in smaller pieces?
I've been using this implementation for months, and while it's still incomplete, it's a massive improvement over not having support for Go 1.18. I would suggest merging this sooner, to improve user experience in the next stable build, and then I'll open a new issue for the remaining unresolved problems. |
Could you probably have a look at the conflicts? |
* Convert anon to named contexts for compatibility with overrides. * Avoid false positives when scoping javadoc bullets. * Use `prototype` to reduce repetition.
Features: * [x] Support for ~. * [x] Support for `any` and `comparable`. * [x] Support for type unions. * [x] Support for general interfaces (with type unions). * [x] Support for type parameters in generic functions. * [x] Support for type parameters in method receivers of generic types. * [x] Support for type arguments following type names. * [x] Support for type parameters in type declarations. * [x] Support for embedded parametrized types in struct definitions. * [x] Support for type arguments between function name and function arguments. * [x] Support for `invalid.illegal` for dangling `)]}`. * [x] Slightly better meta scopes in type definitions. Known regressions: * Embedded interfaces are no longer scoped as "inherited class", due to ambiguities. * "Namespace" idents such as `ident.` no longer support a comment between identifier and dot. * Last ident in parens followed by opening paren is no longer scoped as a function call, due to false positives and ambiguities. Known issues and limitations: * Detection of type parameter lists and type argument lists is incomplete. In some places it relies on regex lookahead, which doesn't work across multiple lines. There are also edge case ambiguities which are supported by the Go compiler but not by our syntax.
Co-authored-by: deathaxe <deathaxe@web.de>
Co-authored-by: deathaxe <deathaxe@web.de>
Co-authored-by: deathaxe <deathaxe@web.de>
Edit: disregard, I meant a different one. |
This confuses me. The issue seems to be about syntax highlighting inside docblocks, for stuff like |
Sorry, disregard. I meant a different one. I'll look it up. |
Found it, there was some discussion in PR 3291 about the use of |
I'd further like to suggest changing: # ...
set: [one, two] to: # ...
set:
- one
- two as it's easier to read. Also, what about using linebreaks for vars as in: keyword: |-
(?x:
\b(?:
break|
case|chan|const|continue|
default|defer|
else|
fallthrough|for|func|
go|goto|
if|import|interface|
map|
package|range|
return|
select|struct|switch|
type|
var
)\b
) or those longer match patterns for word groups. Maybe you could add comment block begin&end scopes to the folding rules in the fold metadata. Maybe add a test for the indentation rules and symbols. Just a heads up, tests for negative symbols are broken, i.e. using the CI test binary and ... we could do all this later though, too. |
For For long regexes that are lists of words such as predeclared_type: |-
\b(?x:
- bool
+ any
+ |bool
|byte
+ |comparable
|complex64
|complex128
...
)\b ...as opposed to: - predeclared_type: \b(?:byte|complex64|complex128| ...
+ predeclared_type: \b(?:any|bool|byte|comparable|complex64|complex128 ... So if we're changing variable lists, it should be by flattening them vertically, IMO. |
I agree, one-liners beeing well suited in case they fit into 80 chars boundary. Multi-line form got a bit more popular just because of many contexts being pushed at once in syntaxes like Java or JavaScript. They may produce more readable code, if context names get longer. I've tried to keep with one style recently, but there's no strong convention about it, nor a need for one.
Agree. With a look at Ruby, which makes already use of it, and some experiments with applying it to PHP, I'd be a bit concerned about the huge amount of lines added - especially for syntaxes with insane long lists of builtin funcs/vars/... (such as PHP). |
Been using this for a month or so. Great work! |
@patrickrgaffney @jakelucas Any comments? |
I can only assume I've been pinged here by mistake |
Features:
~
.any
andcomparable
.invalid.illegal
for dangling)]}
.Known regressions:
ident.
no longer support a comment between identifier and dot.Known issues and limitations:
Misc tweaks in comments:
prototype
to reduce repetition.Closes #3281.