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

Crash on Windows due to improper backslash handling in autojump_match.py #436

Open
agorgl opened this issue Jul 31, 2016 · 8 comments · May be fixed by #561
Open

Crash on Windows due to improper backslash handling in autojump_match.py #436

agorgl opened this issue Jul 31, 2016 · 8 comments · May be fixed by #561

Comments

@agorgl
Copy link
Contributor

agorgl commented Jul 31, 2016

Executing any normal autojump command in Windows leads to this:

Traceback (most recent call last):
  File "C:\Program Files (x86)\CowShell\Vendor\AutoJump\bin\\autojump", line 320, in <module>
    sys.exit(main(parse_arguments()))
  File "C:\Program Files (x86)\CowShell\Vendor\AutoJump\bin\\autojump", line 314, in main
    ['.'])))
  File "C:\Program Files (x86)\CowShell\Vendor\AutoJump\bin\autojump_utils.py", line 42, in first
    return it.next()
  File "C:\Program Files (x86)\CowShell\Vendor\AutoJump\bin\autojump_match.py", line 86, in <lambda>
    flags=regex_flags,
  File "F:\Programs\Python\lib\re.py", line 146, in search
    return _compile(pattern, flags).search(string)
  File "F:\Programs\Python\lib\re.py", line 251, in _compile
    raise error, v # invalid expression
sre_constants.error: unexpected end of regular expression
ECHO is off.

The highlighted change here: 7c7865e#diff-4b97c2fd2ae952c567c1646bc80e5d43L78
lead to the dysfunction in Windows systems.

The proposed solution is basically changing the lines 78-80 in autojump_match.py to:

    sep = '\\\\' if os.sep == '\\' else os.sep
    regex_no_sep = '[^' + sep + ']*'
    regex_no_sep_end = regex_no_sep + '$'
    regex_one_sep = regex_no_sep + sep + regex_no_sep

(Conditionally setting the separator value used in the regex construction)

@wting
Copy link
Owner

wting commented Aug 1, 2016

Hey thanks for the detailed bug report!

Unfortunately I don't have access to a Windows machine. Can you update these Windows tests such that they reproduce the error you have? That way we can prevent future regression bugs.

@glucas
Copy link

glucas commented Aug 25, 2016

Just trying out autojump on Win10 and hit this. The proposed patch seems to work for me.

@glucas
Copy link

glucas commented Aug 29, 2016

@wting If the tests are not actually executed on Windows, won't the match_consecutive tests pick up the wrong os.sep? The tests won't actually fail unless os.sep is a backslash.

@wting
Copy link
Owner

wting commented Aug 29, 2016

You can mock it out for tests like so:

In [1]: import os, mock

In [2]: os.sep
Out[2]: '/'

In [3]: with mock.patch.object(os, 'sep', '\\'):
   ...:     print(os.sep)
   ...:     
\

@willyd
Copy link

willyd commented Oct 28, 2016

I had the same problem and the suggested fix solved the problem. I guess that an alternative solution would be to run the tests on a windows CI platform such as appveyor.

floydpink pushed a commit to floydpink/autojump that referenced this issue Jan 29, 2017
glucas added a commit to glucas/autojump that referenced this issue May 26, 2017
glucas pushed a commit to glucas/autojump that referenced this issue Feb 5, 2018
glucas pushed a commit to glucas/autojump that referenced this issue Feb 5, 2018
glucas added a commit to glucas/autojump that referenced this issue Feb 5, 2018
floydpink pushed a commit to floydpink/autojump that referenced this issue Feb 20, 2018
regisbsb pushed a commit to regisbsb/autojump that referenced this issue Nov 27, 2018
@com314159
Copy link

win 10 64 , the same issue. change autojump_match.py worked. thank you.

@shukebeta
Copy link

shukebeta commented Mar 19, 2020

Window 10 professional, I met this issue today....and the suggested patch works like a charm! Many thanks. I just wonder why the patch is not merged into the master while four years have been passed?

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

Successfully merging a pull request may close this issue.

7 participants
@glucas @wting @willyd @shukebeta @agorgl @com314159 and others