add Go branch scope support

[josharian]

Not all the behavior is perfect (I'm looking at you, pour and drink), but it's better than not existing.

The perfect is the enemy of the good.

Checklist

• I have added tests
• [-] I have updated the docs and cheatsheet
• [-] I have not broken the cheatsheet

[josharian]

outstanding:

• figure out how to get drink, pour, and chuck all to do the right thing at the same time (sigh)
• add tests once behavior is correct
• move TODOs to some personal list, or just implement; maybe move over condition at the same time?

note also this is a potentially interesting case for #1631

[pokey]

Looking at your scope visualizer screenshots from slack, it appears that the problem is that tree-sitter-go includes the trailing newline in the node's range. I'd recommend using our shrink regex predicate operator to exclude it. Hopefully that will fix your various issues.

Keep in mind that "chuck branch" will leave an empty line, but pretty much every other scope type has the same problem, and we're planning to fix all tree-sitter whitespace removal using the #855 hammer

[josharian]

Range:

Removal:

Iteration:

[pokey]

From your screenshots, it appears the newline is still being included.

Fwiw I do wonder if we want to create a "trim" query predicate operator so we don't have to keep reinventing the wheel with whitespace / newline trimming

[josharian]

PTAL at these visualizer screenshots. If it looks OK, I'll add more tests and then you can review the code.

I remain unhappy with the pour/drink behavior, but I'm realizing that fixing that is a much, much larger project that touches other scopes, and I'm pretty ready to get the basic functionality in.

[pokey]

had a quick look and tests / visualizer looks good! left some minor comments; didn't review the .scm thoroughly as it's a bit tough to read without autoformatter

[josharian]

added some tests. unfortunately, the behavior in

packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/languages/go/cloneBranch2.yml

and

packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/languages/go/cloneBranch3.yml

is bad: it generates invalid code.

I am not sure how to fix it. Looks like a line number off-by-one?

Because they're hard to read (maybe I really should finish up #1805? there was so little enthusiasm for it), here's an example.

Start:

if x {
	x++
} else if y {
	y++
} else {
	x--
}

Then "clone branch" with cursor at the beginning of the y++ line. Result:

if x {
	x++
else if y {
	y++
}
} else if y {
	y++
} else {
	x--
}

Best would be something like:

if x {
	x++
} else if y {
	y++
} else if y {
	y++
} else {
	x--
}

But at a minimum, I'd expect valid syntax, so maybe:

if x {
	x++
}
else if y {
	y++
} else if y {
	y++
} else {
	x--
}

[josharian]

Sigh. The precommit failure is due to typescript code I added to the playground to experiment with branch scope behavior. Can we extent the playground exemption to include typescript?

[josharian]

Fixed clone. It's not quite perfect, but it's close enough. The weird setting of the insertion delimiter on @branch.start.startOf instead of just @branch is because we evaluate the insertion delimiter query before we rewrite .start.startOf, and it complains because plain old @branch doesn't exist at that point. We could reorder, or be more forgiving...the relevant code says:

      // FIXME: We could allow some predicates to be forgiving,
     // because it's possible to have a capture on an optional nodeInfo.
     // In that case we'd prob just return `true` if any capture was
     // `null`, but we should check that the given capture name
     // appears statically in the given pattern.  But we don't yet
     // have a use case so let's leave it for now.

But let's just get this in for now?

[josharian]

PTAL

[josharian]

I believe this is ready for review.

[josharian]

Ping.

[pokey]

Looks good with some minor comments!

[pokey]

Is this necessary? I think a space is the default insertion delimiter.

Suggested change

(#insertion-delimiter! @branch.start.startOf " ")

[pokey]

As above

Suggested change

(#insertion-delimiter! @branch.start.startOf " ")

[pokey]

I'm pretty sure you don't need the .startOf / .endOf if they're not overlapping and you can rely on the order

Suggested change

	"else" @branch.start.startOf
	alternative: (if_statement
	consequence: (block) @branch.end.endOf
	"else" @branch.start
	alternative: (if_statement
	consequence: (block) @branch.end

[pokey]

as above

Suggested change

(#insertion-delimiter! @branch.start.startOf " ")

[pokey]

Made the final small tweaks to get this one merged in

[josharian]

Made the final small tweaks to get this one merged in

Thanks! Sorry I'm such a mess at the moment. Really appreciated.

[pokey]

Not at all!