馃悰 Fix sourcing of completion path for Git Bash #801
Conversation
svlandeg
added
bug
| @@ -100,13 +100,13 @@ def install_bash(*, prog_name: str, complete_var: str, shell: str) -> Path: | |||
| # It seems bash-completion is the official completion system for bash: | |||
| # Ref: https://www.gnu.org/software/bash/manual/html_node/A-Programmable-Completion-Example.html | |||
| # But installing in the locations from the docs doesn't seem to have effect | |||
| completion_path = Path.home() / f".bash_completions/{prog_name}.sh" | |||
| completion_path = Path.home() / ".bash_completions" / f"{prog_name}.sh" | |||
There was a problem hiding this comment.
Note that this particular fix is not required to solve the issue on my end (in fact, it makes no difference). I just think it's safer to always use the Pathlib separator / instead of the literal "/".
There was a problem hiding this comment.
It really makes no difference because Pathlib perfectly understands / in paths 馃槉 it's designed to be cross-platform
There was a problem hiding this comment.
Yea I think you're right - it makes no difference because Pathlib will deal with the literal string correctly, too. Honestly I'd still prefer the edit for esthetic reasons (part of the same path is using /, the other part is using "/") but I don't feel strongly. Will leave it up to Tiangolo - and either way as I said this is not actually related to the bug fix.
There was a problem hiding this comment.
Hey, I also think it's more esthetic this way 馃槈 just pointing out it doesn't really make it any safer
| rc_path = Path.home() / ".bashrc" | ||
| rc_path.parent.mkdir(parents=True, exist_ok=True) | ||
| rc_content = "" | ||
| if rc_path.is_file(): | ||
| rc_content = rc_path.read_text() | ||
| completion_init_lines = [f"source {completion_path}"] | ||
| completion_init_lines = [f"source '{completion_path}'"] |
There was a problem hiding this comment.
It's safe to quote in posix, too.
In fact it's the right thing to do, even on posix, because the home directory might contain spaces
However, I recommend quoting using shlex.quote, which will also cover other corner cases
My system
When running
on my Windows system with Git Bash, Typer fetches
Path.home()which isC:\Users\Sofie. This is correct in itself, but Git Bash can't work directly with paths that contain backslashes. After installation,.bashrccontainsand after rebooting the terminal, it will complain:
To get this to work with Git Bash,
.bashrcshould instead containand then it works - no error upon starting the console, and the completion feature works.
TODO
This PR needs to be carefully checked on a "normal" Bash system to double check that the single quote doesn't harm any other environments. I wouldn't think so, but I'm not 100% sure.