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

Bug fix: infinite loop when the current working directory doesn't exist #751

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

romkatv
Copy link
Contributor

@romkatv romkatv commented Jul 19, 2020

To reproduce:

mkdir /tmp/foo
cd /tmp/foo
rmdir /tmp/foo
exec zsh -f
source ~/zsh-syntax-highlighting/zsh-syntax-highlighting.zsh

Press a and zle will hang.

To reproduce:

    % mkdir /tmp/foo
    % cd /tmp/foo
    % rmdir /tmp/foo
    % exec zsh -f
    % source ~/zsh-syntax-highlighting/zsh-syntax-highlighting.zsh

Press 'a' and zle will hang.
@danielshahaf danielshahaf added this to the 0.9.0 milestone Jul 19, 2020
@danielshahaf
Copy link
Member

Thanks. Triaged. Milestoned 0.9.0 because it's not a regression, though realistically I'd like to see it in 0.8.0 or, failing that, 0.8.1. No time to test or review right now, sorry.

@phy1729
Copy link
Member

phy1729 commented Jul 20, 2020

@romkatv thank you for the minimal reproducer. Thoughts on

diff --git a/highlighters/main/main-highlighter.zsh b/highlighters/main/main-highlighter.zsh
index e59c61c..24d0b75 100644
--- a/highlighters/main/main-highlighter.zsh
+++ b/highlighters/main/main-highlighter.zsh
@@ -1210,6 +1210,9 @@ _zsh_highlight_main_highlighter_check_path()
   # Check if this is a blacklisted path
   if [[ $expanded_path[1] == / ]]; then
     tmp_path=$expanded_path
+  elif ! [[ -e . ]]; then
+    # If $PWD doesn't exist, relative paths are impossible
+    return 1
   else
     tmp_path=$PWD/$expanded_path
   fi

Am I missing an edge case?

@romkatv
Copy link
Contributor Author

romkatv commented Jul 20, 2020

No time to test or review right now, sorry.

This is not blocking me in any way (I never delete my current directory), so take your time.

@romkatv thank you for the minimal reproducer. Thoughts on

Your version of the patch has a race condition (so it can still go into an infinite loop) and doesn't colorize any files if the current directory doesn't exist (even though some files could still be colorized). That said, I've very little emotional investment in this PR one way or another, so if you decide to apply your patch I won't argue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants