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

Path shortening is not working in bash 4.3 #289

Closed
ecksun opened this issue Mar 12, 2014 · 16 comments
Closed

Path shortening is not working in bash 4.3 #289

ecksun opened this issue Mar 12, 2014 · 16 comments
Labels
bash Related to Bash specific implemetation bug path Related to current working directory lookup or display

Comments

@ecksun
Copy link
Contributor

ecksun commented Mar 12, 2014

If I stand in my home directory or any child thereof the path to my home is not shortened to ~

The reason for this is a change in how bash handles expansion in pattern substitutions:
http://lists.gnu.org/archive/html/bug-bash/2014-03/msg00036.html

The incorrect behavior is because of:

local p="${PWD/#$HOME/~}"

located here:
https://github.com/nojhan/liquidprompt/blob/master/liquidprompt#L539

I can think of two ways of fixing this for bash 4.3:

local p="${PWD/#$HOME/'~'}"
local p="${PWD/#$HOME/\~}"

However neither works on bash 4.2, I also do not know how to deal with zsh.

One could also probably use compability mode:

shopt -s compat42

or simply check for the bash version currently used.

I do not mind implementing the fix, however I do not know how to do it in a clean manner so suggestions are welcome.

LudovicRousseau added a commit to LudovicRousseau/liquidprompt that referenced this issue Mar 14, 2014
Fixes issue liquidprompt#289

The solution was described in
https://lists.gnu.org/archive/html/bug-bash/2014-03/msg00044.html

I had to remove the double quotes. I have no problem with spaces
characters in directories. I hope it is OK.
@LudovicRousseau
Copy link

I also have the same problem on Debian testing.

I don't think that using the compatibility mode is the correct long term solution.
I propose a simple patch in LudovicRousseau@80d811c

@JForst
Copy link

JForst commented Mar 14, 2014

as ecksun pointed out this simple patch is neither working in bash 4.2 nor in zsh
so I tried to avoid this by checking the bash version:

https://github.com/JForst/liquidprompt/commit/995dcaca530fe13066aa8843cc5aead58ab06595
I tested this with zsh and bash 4.2/ 4.3
Comments?

@LudovicRousseau
Copy link

I had not tested with bash 4.2. Only with bash 3.2.51 on Mac OS X.

Your check is only for bash 4.3. The behaviour will change again with bash 4.4? Or should we use '~' for any bash >= 4.3?

@JForst
Copy link

JForst commented Mar 14, 2014

As I understand the discussion on http://lists.gnu.org/archive/html/bug-bash/2014-03/msg00036.html this is a fix for a bug (first answer in the discussion).
Given this as a fact this should be valid for any bash >= 4.3.

my check is for major number 4 and minor >= 3, so any release of 4 should work. Maybe we should also consider major number >=4

@LudovicRousseau
Copy link

I miss-readed your check, sorry. You are right.

According to another email in the same thread https://lists.gnu.org/archive/html/bug-bash/2014-03/msg00044.html using '~' should work for <=bash-4.2 and >=bash-4.3. So any version of bash except 4.2.x?

@JForst
Copy link

JForst commented Mar 14, 2014

I think you are right, I do understand this a <=bash-4.2 and >=bash-4.3

but on my MAC-OS with an older version of the bash, this does not work.

[forst:'~'] $ bash --version
GNU bash, version 3.2.48(1)-release (x86_64-apple-darwin12)
Copyright (C) 2007 Free Software Foundation, Inc.

Maybe it is best to keep it with >=bash-4.3, since there where no problems
reportet until bash-4.3.

But the name of the variable _LP_SHELL_bash43 is clearly misleading. Maybe
_LP_SHELL_bash_gt_42 is better.

@ecksun
Copy link
Contributor Author

ecksun commented Mar 14, 2014

If the variable name is _LP_SHELL_bash_gt_42 (which I think it should be), perhaps the check should be

[[ $bmajor -ge 4 && $bminor -ge 3 ]]

Just to be clear, is this a bug only in bash 4.2? in bash > 4.2 we should be using the escaped version?
Which would implicitly mean that zsh also uses bash's "faulty" behaviour?

If so, I would like to rewrite my patch somewhat as the code now suggest that it is bash >=4.3 that does is buggy.

ecksun@8906e0f

@LudovicRousseau
Copy link

The check must be smarter than that or bash version 5.0 to 5.2 will not detected correctly.
Something like:
[[ $bmajor -ge 4 && $bminor -ge 3 || $bmajor -ge 5]]

@ecksun
Copy link
Contributor Author

ecksun commented Mar 14, 2014

You are right but I would rather do [[ $bmajor -eq 4 && $bminor -eq 2 ]], that is test for the version with the faulty behaviour (as you suggested previously). However I don't have any other bash version than 4.2 and 4.3 to test with.

@JForst
Copy link

JForst commented Mar 17, 2014

As said before, I think we shouldn't change the behavior which worked
before.
I happen to have some 'old' bashes, where

local p="${PWD/#$HOME/'~'}"

just breaks: e.g

[forst@osi:'~'] $ bash --version
GNU bash, version 3.2.51(1)-release (sparc-sun-solaris2.10)
Copyright (C) 2007 Free Software Foundation, Inc.

or this:

[forst:'~'] $ bash --version
GNU bash, version 3.2.48(1)-release (x86_64-apple-darwin12)
Copyright (C) 2007 Free Software Foundation, Inc.

@LudovicRousseau
Copy link

How does it break?

Note that the code I proposed is:
local p=${PWD/#$HOME/'~'}
and not
local p="${PWD/#$HOME/'~'}"

You must remove the double quotes " " or you get the single quotes ' ' in the result.

@JForst
Copy link

JForst commented Mar 17, 2014

Oh, sorry - my fault!

you are right. your code is working on my bash3.2 versions.
my apologoies

@dolmen
Copy link
Collaborator

dolmen commented Mar 18, 2014

Please check what happens when $PWD contains spaces...

@LudovicRousseau
Copy link

It works with spaces in directory names:

[rousseau:~/foo bar baz/pouet] $ echo $PWD
/Users/rousseau/foo bar baz/pouet
[rousseau:~/foo bar baz/pouet] $ bash --version
GNU bash, version 3.2.51(1)-release (x86_64-apple-darwin13)
Copyright (C) 2007 Free Software Foundation, Inc.

ecksun pushed a commit to ecksun/liquidprompt that referenced this issue Mar 19, 2014
Pattern substituion in bash 4.2 was faulty, which caused tilde to not be
expanded in certain cases. This behaviour was previously relied on, now
we are only using that behaviour as a fallback on bash 4.2.

This resolves issue liquidprompt#289
ecksun pushed a commit to ecksun/liquidprompt that referenced this issue Mar 29, 2014
Pattern substituion in bash 4.2 was faulty, which caused tilde to not be
expanded in certain cases. This behaviour was previously relied on, now
we are only using that behaviour as a fallback on bash 4.2.

This resolves issue liquidprompt#289
dolmen pushed a commit that referenced this issue May 30, 2014
Pattern substituion in bash 4.2 was faulty, which caused tilde to not be
expanded in certain cases. This behaviour was previously relied on, now
we are only using that behaviour as a fallback on bash 4.2.

This resolves issue #289
@ecksun ecksun closed this as completed Jun 9, 2014
pkkolos pushed a commit to pkkolos/liquidprompt that referenced this issue Jun 28, 2014
Pattern substituion in bash 4.2 was faulty, which caused tilde to not be
expanded in certain cases. This behaviour was previously relied on, now
we are only using that behaviour as a fallback on bash 4.2.

This resolves issue liquidprompt#289

Conflicts:
	liquidprompt
@geoffreywiseman
Copy link

I'm trialing liquidprompt on bash 4.3.30 on OS X 10.10 and it seems like I am seeing this same behaviour -- when I'm in a directory under my home directory, I don't get the path shortened with a tilde.

It looks like the solutions discussed in this thread weren't adopted? I can see liquid prompt in head still shows local p="${PWD/#$HOME/~}", but the issue is marked closed?

@ecksun
Copy link
Contributor Author

ecksun commented Oct 31, 2014

The fix should be in the develop branch, I have been using that for quite some time without issues.

However I'm not sure how the release-schedule looks like for liquidprompt, it was a while since master was touched.

@dolmen dolmen added the path Related to current working directory lookup or display label Jan 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bash Related to Bash specific implemetation bug path Related to current working directory lookup or display
Projects
None yet
Development

No branches or pull requests

5 participants