Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix/camel case autocompletion #423

Merged
merged 5 commits into from
Apr 8, 2018
Merged

Fix/camel case autocompletion #423

merged 5 commits into from
Apr 8, 2018

Conversation

rberenguel
Copy link
Contributor

Thanks @olafurpg for the Kebabizer :)

@jvican jvican added bug A defect or misbehaviour. cli nailgun community Any change or proposal that is contributed by the Open Source Community. and removed nailgun labels Apr 7, 2018
jvican
jvican reviewed Apr 8, 2018
View reviewed changes
Copy link
Contributor

@jvican jvican left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a spurious CI error, let's wait until you address the commend and see if it goes green. Thanks!

frontend/src/main/scala/bloop/cli/completion/Case.scala Outdated
sb.toString
}

def kebabizeArg(arg: Arg): Arg = arg.copy(extraNames =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we put the body of this method inside curly braces and move the implementation of extraNames to a new val? 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure!

jvican
jvican approved these changes Apr 8, 2018
View reviewed changes
Copy link
Contributor

@jvican jvican left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a pity we don't yet have tests for autocompletion. In any case, thanks for this change -> 🥇

@jvican jvican merged commit 684942e into scalacenter:master Apr 8, 2018
@rberenguel rberenguel deleted the fix/camel_case_autocompletion branch April 8, 2018 16:46
@rberenguel
Copy link
Contributor Author

@jvican actually I was thinking about that on Friday, I'm not sure how autocompletion could be tested completely (covering all possible shells). Any idea? I could create the issue if there is any possible approach

@jvican
Copy link
Contributor

jvican commented Apr 8, 2018

@rberenguel Covering bash and zsh autocompletions is a good start. The good thing is that you can create a simple test suite for each one that uses the Autocomplete command defined in Commands.scala. You can have different input and simulate what would be the expected response.

@jvican jvican mentioned this pull request Apr 8, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jvican jvican jvican approved these changes

Assignees
No one assigned
Labels
bug A defect or misbehaviour. cli community Any change or proposal that is contributed by the Open Source Community.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rberenguel @jvican