-
Notifications
You must be signed in to change notification settings - Fork 646
Conversation
@goenning, |
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.
Love the idea. Left a suggestion on some refactoring ideas.
src/goRunTestCodelens.ts
Outdated
return testFunctions.map(func => { | ||
let showReferences: Command = { | ||
title: 'run test', | ||
command: 'go.test.function', |
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.
Instead of creating a new command, I'd prefer re-using the existing go.test.cursor
.
You can pass the arguments as { functionName: func.name}. And update
testAtCursorto look for
args.functionName`. If it exists use it, else use the existing logic to find the test function under the cursor
src/goRunTestCodelens.ts
Outdated
import { getTestFunctions } from './goTest'; | ||
|
||
export class GoRunTestCodeLensProvider implements CodeLensProvider { | ||
public provideCodeLenses(document: TextDocument, token: CancellationToken): CodeLens[] | Thenable<CodeLens[]> { |
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.
How about adding a codelens at the package level as well? Every go file has a package
statement up top. So we can add a code lens there and map it to the go.test.package
command
const range = pkg.location.range; | ||
return [ | ||
new CodeLens(range, { | ||
title: 'run package tests', |
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.
2 cents suggestion: s/run/Run. The same for run file tests
, run test
.
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.
How about Run tests in package
, Run tests in file
and Run test
?
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.
I personally prefer lowercase + shorter text to not get much attention. It should be as unobtrusive as possible so devs can focus on code an not on these actions. It's like this on C# codelens
Next enhancement could be debug test
😄
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.
as unobtrusive as possible
well, when you put it that way it makes sense.
Next enhancement could be debug test
There is already an ask for it #879 :)
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.
Left a comment on the text of the codlens, everything else looks good. Thanks!
This feature is now out in the latest update (0.6.60) |
Cool feature! Any objection to a feature level config option? I can imagine wanting to disable this feature without disabling CodeLens generally. |
For references codelens, we added the setting, because For the run and debug tests, there is no downside perf wise and it doesnt hinder any workflow so didnt add the setting. (Also our settings are really getting big) Any particular reason you would want to turn it off? |
Perhaps the user only uses keyboard shortcuts for this and doesn't want an editor row occupied by the hints for every function. Vertical editor space is often precious. |
I guess it's up for discussion, but I share same feelings about the amount of settings the extension already has. You can disable it on workspace level, which should be roughly the same as a feature level setting. The only downside I can see is if your workspace has code from multiple languages and you want to disable codelens just for Go. |
Lets wait for some more feedback. If there is more ask for setting on each type of codelens, then we can introduce it later. |
Personally, I think this feature is awesome. I hope we can have more codelens integration! |
Same feature as Test at Cursor, but in this case there is no need to select anything.
It's based on C# VSCode extension.
What you think about it?
I didn't provide any flag to disable it, because it's possible to do so with standard editor flag:
"editor.codeLens": false
.