Hide `if err != nil { return err}` like goland

[guettli]

I am evaluating vscode and goland at the moment. This feature of goland is nice:

Only if I expand the block, I see

This feature of goland reduces the amout of characters which my eyes need to parse. If you would sum all these characters which my eyes don't need to parse per day, this would be a lot. A lot less cognitive load.

It would be great, if vscode would support this feature, too.

[hyangah]

@guettli Thanks for the feedback! Can you help us understand exactly what aspect of the folding feature in goland you like?

• Folded by default?
• Unlike VSCode Go that leaves the closing } when folded, goland completely hides {}?
• Or presenting helpful collapsed text e.g. ": err ..." when folded?

(VSCodeGo/gopls currently doesn't utilize the collapsedText feature yet and shows "..." cc @golang/tools-team )

[taqqanori]

l'm just thinking the same, and created an extension, although it uses built-in editor.fold command (leaves } unfolded, no helpful collapsed text). https://marketplace.visualstudio.com/items?itemName=taqanori.hide-error-cases

[gopherbot]

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

[jogly]

Folded by default? Unlike VSCode Go that leaves the closing } when folded, goland completely hides {}? Or presenting helpful collapsed text e.g. ": err ..." when folded?

Hopefully resurrecting this instead of making a new feature request 🙏 . @hyangah - your first two points are what would be most useful to me. The third would be nice to have. The second point is the most painful of the three; the dangling brace makes folding three lines into two not worth the effort. FWIW, I don't care if the braces are hidden or not; it's the trailing brace on it's own line that hurts readability. For example, the following are equivalently useful to me:

if err != nil {...}

if err != nil : err ⤴︎

[suzmue]

The LSP spec also now has (proposed?) support for collapsedText in folding range results as of 3.17.0 https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_foldingRange

[guettli]

I think foldingRange of LSP should be supported now by vscode.

It would be great, if you could fold/unfold lines which return errors.

[firelizzard18]

I'd like to take a look at implementing this, though I'm not sure when I'll have time (I'm a community contributor despite the "member" tag).

[sebastian-philipp]

Why not also hide the if err != nil {...}part and reduce it to: err ⤴︎

like from:

data, ok := something.Data["data"]
if !ok {
	return nil, errors.New("missing data")
}

cfg, err := GetCfg(data)
if err != nil {
	return nil, errors.Wrap(err, "failed to get cfg")
}

scheme := runtime.NewScheme()
err = clientgoscheme.AddToScheme(scheme)
if err != nil {
	return nil, errors.Wrap(err, "failed to add client-go scheme")
}
err = other.AddToScheme(scheme)
if err != nil {
	return nil, errors.Wrap(err, "failed to add other scheme")
}

client, err := something.New(cfg, scheme)
if err != nil {
	return nil, errors.Wrap(err, "failed to create client")
}

to:

data, ok := something.Data["data"]
if !ok {
	return nil, errors.New("missing data")
}

cfg, err := GetCfg(data)
err ⤴︎

scheme := runtime.NewScheme()
err = clientgoscheme.AddToScheme(scheme)
err ⤴︎
err = other.AddToScheme(scheme)
err ⤴︎
client, err := something.New(cfg, scheme)
err ⤴︎

[firelizzard18]

Personally I find that a lot less readable. I could possibly see folding it into the previous line but folding it like you suggest makes it a lot less obvious to me at a glance what's happening.

[h9jiang]

Hi, I did some research based on the discussion above and want to experiment with how this can be implemented.

Current behavior, the code snippet below will be folded as two lines.

// before folded
if err := function(); err != nil {
   return err
}

// folded
if err := function(); err != nil {...
}

Based on comment desired behaviors are:

1. when folded, the } should not occupied it's own line. The code snippet above should be folded as if err := function(); err != nil {...} in one line instead of two lines.
2. error handling should be folded by default.
3. nice to have: provide helpful collapsed text.

Here are some limitations I found from VSCode.

1. collapsedText is now supported by LSP FoldingRangeClientCapabilities, but not supported by VSCode. FR in microsoft/vscode-languageserver-node#1068 microsoft/vscode#70794.
2. lineFoldingOnly being marked as true in vscode-languageserver-node meaning the gopls will only send line info instead of further detailed starting character and ending character.
3. The current foldingRangeKind set is very limited. We want to introduce more kind to help us understand the code better.

Let's come back to the desired behavior:

1. Gopls can return an extra line if the closing } is the only character in that line. Right now, gopls is returning {start: 0, end: 1, kind: unknown}, if gopls can return {start: 0, end: 2, kind: unknown}. The VSCode will be able to fold the code in form of

if err := fmt.Errorf(""); err != nil {...

It's annoying because of the mismatching { and }.

One way to make it better is to include extra } and the end. If the collapsedText is implemented, we can customize the text using ...} (Unfortunately this is blocked until collapsedText microsoft/vscode#70794 is implemented or microsoft/vscode#3352 same line } upon folding is implemented so we get this for free.).

if err := fmt.Errorf(""); err != nil {...}

The other way to improve it is get rid of {. But because the client capabilities lineFoldingOnly = true mentioned above, the language client can only fold entire line instead of part of the line. So this is not doable.

if err := fmt.Errorf(""); err != nil ...

2. Gopls can return extra folding kind information instead of unknown. Like {start: 0, end: 1, kind: error-handling} or simply default-folded. So vscode-go can call editor.Fold on folding range of this type. vscode-go can also trigger this editor.Fold command when a file get opened. So by default, error handling folding range will be folded.

3. Some more thinking need to do in Gopls side when determine what should be considered as a range should be folded by default. Like, gopls should consider the statements inside of error handling block (error handling with one return statement can be folded by default), gopls should consider the function signature (error handling with one return statement and every other return value except for error is zero value can be folded by default) ....

I'm new to the VS Code API, so please feel free to correct me or suggest alternative solutions if I'm off track.

[firelizzard18]

The heuristic I would choose is:

• The function's last return value is error.
• (*IfStmt).Body contains a single statement and that statement is a *ReturnStmt.
• The last (*ReturnStmt).Results is a value/something other than nil.
• Consider a followup CL to add support for variants such as { err = fmt.Errorf(...); return err }.
• It would be nice to validate that the error return value is non-nil (e.g. excluding if err == nil { return err }) but maybe that can be another followup.

Personally I wouldn't worry about the other return values. IMO the error conditional in the following example should be folded. In summary: "If a conditional checks err != nil and returns an error, it is considered to be an error handling statement regardless of other return values." My logic is, there are circumstances where it makes sense to return non-zero values for other parameters from an error handling statement, so gopls should respect that.

func Foo(x int) (int, error) {
	err := Bar(x)
	if err != nil {
		return -1, err
	}
	return x, nil
}

[eihigh]

Since the Go team has decided not to improve the language's error handling syntax, editor support for this has become even more important. Is anyone here still interested in this issue? https://go-review.googlesource.com/c/website/+/673615

[adonovan]

This seems reasonable as an optional feature. If someone sends a CL to use FoldingRange's CollapsedText feature, I'll review it.

[h9jiang]

Hi eihigh@, I think this is a feature that definitely worth implementing. But as I mentioned above in the previous comment, we are now blocked by upstream.

To summarize from the previous comment:

if microsoft/vscode#70794 and https://github.com/microsoft/vscode-languageserver-node/issues/1068 is fixed, the vscode-go and gopls can make folding looks like:

// folded
if err := function(); err != nil {err ⤴︎}

if https://github.com/microsoft/vscode/issues/3352 is fixed, the vscode can make the folding looks like:

// folded
if err := function(); err != nil {...}

I'm not sure whether there is a tracking issue for vscode partial folding or not, but based on my understanding, vscode only support line folding. vscode-languageserver-node

After we make the folding looks better, the gopls can make improvement of the folding type by returning folding kind telling the client this is a folding for error handling, please keep it folded by default. So the vscode-go extension can fold it by default.

If other language client have support for collapsedText, we can make the gopls change first. At least, other IDE experience would be better while waiting for vscode. :D

If there is any other approaches we can work around this, please suggest and we can explore those options.

[firelizzard18]

I previously said I was interested in working on this but unfortunately I don't think I'll have the bandwidth for a while.

[myaaaaaaaaa]

An alternative approach would be to lower the opacity of err != nil blocks, except for when the mouse hovers over them or when the cursor is inside them.

See the below demonstration for details:

https://github.com/user-attachments/assets/c1351b72-544a-4405-8fea-23ea81ed229f

[ufukty]

@myaaaaaaaaa You might consider that on-hover behaviour because of performance concerns. From my previous experience on developing a dimming extension for vscode, I can tell that the performance impact might be significant enough to annoy user, or cause user to lose focus on rare but important occasions. It might land below the line where the value meets cost. Other hand, skipping only the focused/selected areas provides good balance on that ratio.

[adonovan]

Another possibility would be for the server's semantic tokens query to return an optional modifier for all the tokens in if err != nil { ... } that would allow the client to render it differently somehow. Might be worth a quick experiment.

[firelizzard18]

I think it's valuable to discuss improving folding (this issue) and purely visual/color/opacity changes (#3793) separately, since the implementation of those is different and as I stated in the other issue I can see use cases for implementing both.