-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
.fzf/install fails to install if .bashrc changes from previous installation are commented out #3632
Comments
Thanks for the report. Do you think it would make sense to look for the pattern that is not preceded by diff --git a/install b/install
index 5d031fd..3ca5a80 100755
--- a/install
+++ b/install
@@ -311,7 +311,7 @@ append_line() {
if [ $# -lt 4 ]; then
lno=$(\grep -nF "$line" "$file" | sed 's/:.*//' | tr '\n' ' ')
else
- lno=$(\grep -nF "$pat" "$file" | sed 's/:.*//' | tr '\n' ' ')
+ lno=$(\grep -n "$pat" "$file" | sed 's/:.*//' | tr '\n' ' ')
fi
fi
if [ -n "$lno" ]; then
@@ -349,7 +349,7 @@ echo
for shell in $shells; do
[[ "$shell" = fish ]] && continue
[ $shell = zsh ] && dest=${ZDOTDIR:-~}/.zshrc || dest=~/.bashrc
- append_line $update_config "[ -f ${prefix}.${shell} ] && source ${prefix}.${shell}" "$dest" "${prefix}.${shell}"
+ append_line $update_config "[ -f ${prefix}.${shell} ] && source ${prefix}.${shell}" "$dest" "^[^#]*${prefix}\.${shell}"
done
if [ $key_bindings -eq 1 ] && [[ "$shells" =~ fish ]]; then |
"preceded by a '#'" is not going to be 100% reliable -- if the "#" is inside any kind of quotes, it probably doesn't comment out anything in the current file but that's probably pretty nasty to regex around. As the old saying goes:
That's why I declined to attempt a patch. That said, even just looking for a '#' as the first non-whitespace character of the line would have covered my case and would almost certainly handle 99% of these issues because I'm sure most of them are just like mine and for the same reason. I just commented it out with a "#" as the first character on the line without touching anything else to make sure any updated code would be added to my .bashrc (or wherever the heck it was going) when I reinstalled. As to the regex below, I would not assume that it is anchored at the beginning of the line, only that it's the first non-whitespace character. ssteinerX |
The regex requires that the full prefix does not contain any Anyway, I'm not targeting 100% accuracy with this approach. See, the line could be inside a multi-line string, or inside a function that is never called. We would never know. But it will work 99.99% of the time. The real question is, is this the right thing to do? It's hard to say. If a user manually commented out the line, maybe they want to keep it that way, and we should probably not add another line. We could instead just print the found line to the console, and let the user have a better understanding of what's happening. |
Or you could say: I was about to add "xxxxxxx" to .bashrc and found "xxxxxxx" commented out. Proceed to modify .bashrc? (y/N)? |
man fzf
)Info
Problem / Steps to reproduce
I was reinstalling
fzf
after deciding my package manager's version was too far out of date.I just commented out fzf's .bashrc changes like so:
When running
~/.fzf/install
again, I got the message:Unfortunately, the message was correct, the text was there, but
install
did not notice that the text was commented out.Since the required changes were "already there,"
install
did not make the required changes to insertfzf
into the PATH, load completion, and set key bindings. IOW, even thoughinstall
went through all the steps without error, not noticing that the required text was commented out made the install "fail."After looking at
append_line()
in the install script, I determined that attempting to submit a pull request for a fix would probably waste more of everyone's time than just reporting the issue.The text was updated successfully, but these errors were encountered: