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
Initial implementation for code actions #155
Conversation
run tests |
1803a0c
to
14e0c4a
Compare
params.setRange(range); | ||
params.setContext(new CodeActionContext(Arrays.asList(getDiagnostic(Integer.toString(IProblem.UnusedImport), range)))); | ||
List<? extends Command> commands = server.codeAction(params).join(); | ||
Assert.assertFalse(commands.isEmpty()); |
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.
checking isEmpty is useless when checking the size on next line
params.setRange(range); | ||
params.setContext(new CodeActionContext(Arrays.asList(getDiagnostic(Integer.toString(IProblem.UnterminatedString), range)))); | ||
List<? extends Command> commands = server.codeAction(params).join(); | ||
Assert.assertFalse(commands.isEmpty()); |
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.
checking isEmpty is useless when checking the size on next line
* | ||
*/ | ||
@RunWith(MockitoJUnitRunner.class) | ||
public class CodeActionHandlerTest extends AbstractCompletionBasedTest { |
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'd probably make that class abstract and have separate test classes to check separate code actions
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.
What is the benefit?
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'm expecting we'll see lots of different code actions, cramming everything under the same test class is gonna be messy
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.
A class per code actions is not less messy either. When we have a lot of code actions, we can think of a way to organize the tests. probably around common actions such as unused code etc.
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.
ok. No biggie
14e0c4a
to
52e16d9
Compare
+1 to apply, once the conflict is resolved |
Implements “insert quotes” and remove import code actions. Lays the groundwork for more code actions. Signed-off-by: Gorkem Ercan <gorkem.ercan@gmail.com>
52e16d9
to
4094ddd
Compare
Add tests for the implemented code actions. Signed-off-by: Gorkem Ercan <gorkem.ercan@gmail.com>
4094ddd
to
991bcee
Compare
Implements “insert quotes” and remove import
code actions. Lays the groundwork for more
code actions.
Signed-off-by: Gorkem Ercan gorkem.ercan@gmail.com