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

internals: add path completion w tests #61

Merged
merged 3 commits into from
Mar 29, 2020

Conversation

Kappers
Copy link
Contributor

@Kappers Kappers commented Mar 28, 2020

As per 'Autocompletion broke completion for paths (file, destination, database)' #38

Any input would be appreciated, or style/whatever guidelines for the repo (does that information exist anywhere?) I haven't written tests in a bit too long....

This works, but looks a bit messy for deep directories (long paths) or long file names etc. At least it works!

@codecov
Copy link

codecov bot commented Mar 28, 2020

Codecov Report

Merging #61 into master will increase coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #61      +/-   ##
==========================================
+ Coverage   97.85%   97.91%   +0.05%     
==========================================
  Files          20       20              
  Lines        1028     1055      +27     
==========================================
+ Hits         1006     1033      +27     
  Misses         22       22              
Impacted Files Coverage Δ
bibo/bibo.py 97.82% <ø> (ø)
bibo/internals.py 92.50% <100.00%> (+0.53%) ⬆️
tests/bibo/test_internals.py 100.00% <100.00%> (ø)

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 2bbf83d...851929b. Read the comment docs.

@Nagasaki45 Nagasaki45 changed the base branch from master to dev March 29, 2020 08:51
@Nagasaki45
Copy link
Owner

Looks great!

Note that there's an issue with trailing spaces when trying to expand a nested path, at least in zsh. Apparently that's one of the major issues with supporting path completion in multiple shells. Check out long discussion on pallets/click#1403. Anyway, for our purposes, as a temp solution, this is really good.

2 minor comments:

  • Can you base future PRs on dev, not master. Everything pushed to master is released to pypi :-)
  • Add the number of the issue at the end of the commit to create a link in github from the issue to the commit (with # before the number). If the issue is fixed by the commit add Fix #number.

Adding a note to include these in the dev docs.

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.

2 participants