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.

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

Add bash_completion #308

Merged
merged 6 commits into from
Apr 2, 2020
Merged

Conversation

pablocostass
Copy link

Summary

Adds a bash_completion file for Concuerror with the options from concuerror_options.erl as per requested in #159.

Checklist

  • Has tests (or doesn't need them)
  • Updates CHANGELOG (or too minor)
  • References related Issues

@pablocostass
Copy link
Author

Should I also update the README.md to include instructions on how to install the bash completion?

@aronisstav
Copy link
Member

Thank you very much for the pull request!!

Yes, please, feel free to mention bash completion in README.md! I will take a look at the patch soon!

@codecov
Copy link

codecov bot commented Mar 9, 2020

Codecov Report

Merging #308 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #308   +/-   ##
=======================================
  Coverage   95.72%   95.72%           
=======================================
  Files          12       12           
  Lines        3673     3673           
=======================================
  Hits         3516     3516           
  Misses        157      157           
Flag Coverage Δ
#tests 95.34% <ø> (ø)
#tests_real 84.83% <ø> (ø)
#unit_tests 2.67% <ø> (ø)
Impacted Files Coverage Δ
src/concuerror_options.erl 98.82% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 31d3c20...2ce532d. Read the comment docs.

@kostis
Copy link
Contributor

kostis commented Mar 9, 2020

Thanks @pablocostass for this nice PR!

One comment/concern I have with it though is whether we can find a way to avoid duplication of the list of options that the concuerror script accepts (e.g. have it only on one place) so that it remains consistent after (future) updates.

@aronisstav
Copy link
Member

aronisstav commented Mar 9, 2020 via email

@pablocostass
Copy link
Author

A valid concern @kostis, I can see having to update the file for each option added or modified a bit tedious. Do you have any suggestions on how to avoid duplicating the list of options?

If you ask me, I think as @aronisstav. Most of the options don't have their own data type (which would imply adding more keywords to autocomplete), so adding or removing one is changing two/three lines at most.

I'm no master at testing bash scripts, but if you need an extra pair of hands to do them I can give it a try.

@aronisstav
Copy link
Member

I've started looking into writing some test. While I am struggling to understand how complete works, perhaps someone can enlighten me on how to make it return e.g. a list of all the short or long option names.

@aronisstav
Copy link
Member

@pablocostass
Copy link
Author

What I know of bash completion is thanks to this guide and from looking at rebar3 and kerl's completion. If you are thinking of adding CI testing of/with the bash completion, kerl has a nice config file for both CircleCI and Travis.

In short, you are going to need to know of three variables:

  • COMPREPLY, an array variable of the possible completions to return
  • COMP_WORDS, another array variable made of the words in the current command line, which is used to get the current and previous words thanks to a third variable
  • COMP_CWORD, which points to the current cursor position in COMP_WORDS.

And so, you will normally return a completion based on the previous word (COMP_WORDS[COMP_CWORD-1]) and the current one (COMP_WORDS[COMP_CWORD]).

That completion is generated through the use of compgen and its different options (e.g., to return a word list you would do COMPREPLY=($(compgen -W "word1 word2 word3" -- ${current_word})). You can also fallback to existing completions if you need to, as in the case of wanting to complete the --file option of Concuerror, where I simply let _filedir do the job for me.

@aronisstav
Copy link
Member

More questions:

  1. I was not able to see the useful pieces of info in the links to kerl's CI scripts.
  2. I am looking for documentation for this function: _get_comp_words_by_ref https://stackoverflow.com/questions/60687892/where-can-i-find-documentation-for-get-comp-words-by-ref-function-used-often-in

Alternatively:

  1. I want a bash script that consumes the concuerror autocomplete file and returns the list of long options and from there I hope I will be able to make one for everything I need...

@pablocostass
Copy link
Author

More questions:

  1. I was not able to see the useful pieces of info in the links to kerl's CI scripts.

Oopsie, I'm sorry. Misread the files and thought they were using the completion feature.

  1. I am looking for documentation for this function: _get_comp_words_by_ref https://stackoverflow.com/questions/60687892/where-can-i-find-documentation-for-get-comp-words-by-ref-function-used-often-in

https://github.com/scop/bash-completion/blob/master/bash_completion#L339-L365

Alternatively:

  1. I want a bash script that consumes the concuerror autocomplete file and returns the list of long options and from there I hope I will be able to make one for everything I need...

I'm by no means a regex expert but I could try making one to only get the long options from the file, but (pardon me if I did not understand you correctly) why would you want a script that returns the list of long options from the autocomplete file to make your own one when you can get them all from the long_opts variable?

@aronisstav
Copy link
Member

Thank you for the link, I'll check it in a while.

For the other question, "consumes" was uneccessary poetic :-p I'd like to use some invocation of the actual commands, either by sourcing the script and calling some appropriate version of the _concuerror function and inspecting the relevant variable or something similar.

@pablocostass
Copy link
Author

Maybe these links can be useful: one, two. The first one might be what we want, since it's from a repository of bash-completion files (with testing for them).

I'm a bit busy this afternoon, but I'll see if I can help in any way later.

@aronisstav
Copy link
Member

aronisstav commented Mar 28, 2020

I am back on this today, let's see what I can achieve... :-)

Update 1:
Found this: https://brbsix.github.io/2015/11/29/accessing-tab-completion-programmatically-in-bash/

Update 2:
I am really struggling on MacOS... Instead of the standard _get_comp_words_by_ref function, I seem to have a __git_reassemble_comp_words_by_ref whose behaviour I cannot understand/debug. I am still at it though...

Update 3:
Finally able to make progress :-)

aronisstav added a commit to pablocostass/Concuerror that referenced this pull request Mar 28, 2020
aronisstav added a commit to pablocostass/Concuerror that referenced this pull request Mar 28, 2020
aronisstav added a commit to pablocostass/Concuerror that referenced this pull request Mar 28, 2020
aronisstav added a commit to pablocostass/Concuerror that referenced this pull request Mar 28, 2020
aronisstav added a commit to pablocostass/Concuerror that referenced this pull request Mar 28, 2020
aronisstav added a commit to pablocostass/Concuerror that referenced this pull request Mar 28, 2020
aronisstav added a commit to pablocostass/Concuerror that referenced this pull request Mar 28, 2020
aronisstav added a commit to pablocostass/Concuerror that referenced this pull request Mar 28, 2020
aronisstav added a commit to pablocostass/Concuerror that referenced this pull request Mar 28, 2020
@aronisstav
Copy link
Member

So, I added a few tests for some obvious scenarios and even caught a bug in the listing of long options. I also added a line in the CHANGELOG.

@pablocostass please take a look and let me know if you would like to add any other tests!

@aronisstav aronisstav requested a review from kostis March 28, 2020 22:15
@pablocostass
Copy link
Author

@aronisstav Hey, this is really great! I think those tests are more than enough. Thanks a lot for doing them, as they are very instructive and they will for sure be my reference if I ever need to test bash completion of Erlang programs again 😄

@kostis
Copy link
Contributor

kostis commented Mar 30, 2020

Other than the two (very minor) comments above, this looks good and can be merged.

aronisstav added a commit to pablocostass/Concuerror that referenced this pull request Apr 2, 2020
@aronisstav aronisstav merged commit 11477b1 into parapluu:master Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants