Closed ernstki closed 2 years ago
Thanks @ernstki. @schoettl / @zhelezov, would you care to review ?
Thanks for this fix. Looks good!
I just greped it in the main hledger repo and it's alright there.
Regarding the installation method, I think you can add a note about installation with bash-completion. I guess the completions are still not packaged and installed system-wide automatically.
Thanks! I think your last comment is for @ernstki ? If not I could use simpler instructions.
Re: the bash-completion instructions, I can add that, no problem.
Regarding distro packages shipping with the completion script: there's an existing request to include it in Debian, it just hasn't caught the attention of the maintainer(s) yet. I don't have any knowledge of how it works for other distros at the moment.
Lowest priority, but for my edification as a contributor, are you particular about the branch name for PRs (which will end up in the merge commit message)? The name patch-1
was selected for me by GitHub, with no easy way to change it.
On second thought, I can see some challenges here:
shell-completion
directory to hledger/shell-completion
hledger-ui
's completions are bundled together with hledger
'shledger/shell-completion/README.md
in the hledger repository will further diverge from src/command-line-completion.md
in this repository if I start adding stuff about bash-completionSo it's probably best to just stop here.
I'll defer to your judgement whether you want the docs to reflect
hledger/shell-completion
)shell-completion
)The advantage to doing the latter is it helps out the people whose distro packagers didn't include the completion scripts, and they're stuck on 1.12 or 1.18 or something, which is the case for Debian/Ubuntu right now.
If you're OK with the former (docs only apply to current release and beyond), then this is ready to merge.
The PR branch name is fine - they are transient.
I'll defer to your judgement on the details. I like your attention to packagability, installability, usability, but as someone never quite using/needing shell completions, I'd be fine with conserving dev time here. Could you remind me how the docs are accessed ? Are they/should they be in the hledger manual, which is versioned ?
Well, I was about to say "the manual," but I'm actually unable to find a route to https://hledger.org/command-line-completion.html through any of the navigation sidebar documents without resorting to search. So at the moment it appears to be an orphan, but that's a separate issue. ;)
As a compromise, I added a note about the path to shell-completion
having changed with v1.25, which I hope gets the point across with the fewest words possible. People who are really into bash-completion will be able to figure it out, as I was able, once they know where to find the completion script within the main repository, which was the only real issue here.
Happy to squash commits and re-push if you like a clean history. Apart from that, this is ready to merge if the changes meet with your approval.
That's great, thank you! Squash-merged. I found the link, it's at https://hledger.org/cookbook.html#customising .
Thank you, sir. Cheers!
Looks like the Bash completion script moved into an
hledger
subdirectory. This updates the relevant documentation to point to the new location.I did this with the GitHub web interface, so I didn't exhaustively
grep
the rest of the documentation source for other instances, but if you think there might be others, I'm happy to dig deeper and push another commit to this PR.Lastly, if one has bash-completion already installed, you can
…and let bash-completion take care of the rest. If you feel that including this would add value to the docs, I can add a note about that to the PR as well.