fix(shell-history): fix broken fish config script #22
Conversation
|
@stuartleeks Could you take a look at this? |
| set history_location ~/.local/share/fish/fish_history | ||
| else | ||
| set history_location \$XDG_DATA_HOME/fish/fish_history | ||
| fi |
There was a problem hiding this comment.
Does this work for you without the escaping?
I had to add the escaping back in for the bash history as otherwise the $XDG_DATA_HOME env var is evaluated at the point that oncreate.sh runs and that wasn't set Escaping it ensured that it was evaluated when the .bashrc was run.
There was a problem hiding this comment.
Thanks for your review.
IIUC, replacing EOF to "EOF" allows us to use $ without escaping here.
https://en.wikipedia.org/wiki/Here_document
But from this Wikipedia page, perhaps single quoted 'EOF' might be better?
There was a problem hiding this comment.
Ah - I'll file that under Things I Learned 😄
In terms of single vs double quoting, I think I prefer single as the article suggests
|
|
||
| # Feature-specific tests | ||
| check "check fish config script validity" fish -c "source $HOME/.config/fish/config.fish" | ||
| check "check fish config script content" bash -c "cat $HOME/.config/fish/config.fish | grep -q 'XDG_DATA_HOME'" |
There was a problem hiding this comment.
I have added a test. I believe this test confirms that $XDG_DATA_HOME is present in the script.
eitsupi
changed the title
|
@stuartleeks Sorry to rush you, but I think this is a critical bug and I hope you will consider merging and releasing it. |
.github/workflows/test.yaml
Outdated
| baseImage: "mcr.microsoft.com/devcontainers/base:ubuntu" | ||
| remoteUser: root | ||
| features: | ||
| - dev-tunnels |
There was a problem hiding this comment.
Since the dev-tunnels tests are failing, can you comment out this line?
stuartleeks
left a comment
stuartleeks
left a comment
There was a problem hiding this comment.
LGTM once the dev-tunnels tests are removed. Thanks for the PR!
|
Thanks for merging this! |
|
shell-history 0.0.6 released |
2 participants
I noticed that it does not appear to be the correct fish syntax.