[PATCH] Functions/Misc/relative: use -q with cd

[PATCH] Functions/Misc/relative: use -q with cd

[Daniel Hahler]

From: Daniel Hahler <git@thequod.de>

This skips chpwd function(s), which only add overhead and might cause
unwanted side-effects.
---
 Functions/Misc/relative | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Functions/Misc/relative b/Functions/Misc/relative
index 432f2e2da..db3b5a587 100644
--- a/Functions/Misc/relative
+++ b/Functions/Misc/relative
@@ -8,8 +8,8 @@ emulate -L zsh || return 1
 [[ $1 -ef $2 ]] && print ${3:-.} && return
 
 # The simplest way to eliminate symlinks and ./ and ../ in the paths:
-1=$(cd $1; pwd -r)
-2=$(cd $2; pwd -r)
+1=$(cd -q $1; pwd -r)
+2=$(cd -q $2; pwd -r)
 
 local -a cur abs
 cur=(${(s:/:)2})	# Split 'current' directory into cur
-- 
2.12.2

Re: [PATCH] Functions/Misc/relative: use -q with cd

[Daniel Shahaf]

Daniel Hahler wrote on Thu, Apr 27, 2017 at 22:35:37 +0200:
> +++ b/Functions/Misc/relative
> @@ -8,8 +8,8 @@ emulate -L zsh || return 1
> -1=$(cd $1; pwd -r)
> -2=$(cd $2; pwd -r)
> +1=$(cd -q $1; pwd -r)
> +2=$(cd -q $2; pwd -r)

Would «1=${1:P}» be equivalent?  (If it is, it saves a fork.)

Pre-existing problem: should the function check that cd succeeded?  It
already checked that $1 and $2 exist, but they might be unreadable/unexecutable
by the EUID.

Re: [PATCH] Functions/Misc/relative: use -q with cd

[Bart Schaefer]

On Apr 29,  5:07pm, Daniel Shahaf wrote:
}
} Pre-existing problem: should the function check that cd succeeded?

Aside from stderr messing up the display, are there any consequences
for the completion result if failure is detected rather than ignored?

Re: [PATCH] Functions/Misc/relative: use -q with cd

[Daniel Hahler]

[-- Attachment #1.1: Type: text/plain, Size: 1233 bytes --]

On 29.04.2017 19:07, Daniel Shahaf wrote:

>> +++ b/Functions/Misc/relative
>> @@ -8,8 +8,8 @@ emulate -L zsh || return 1
>> -1=$(cd $1; pwd -r)
>> -2=$(cd $2; pwd -r)
>> +1=$(cd -q $1; pwd -r)
>> +2=$(cd -q $2; pwd -r)
> 
> Would «1=${1:P}» be equivalent?  (If it is, it saves a fork.)

Looks like it, I will adjust the patch before committing anything.


On 29.04.2017 21:47, Bart Schaefer wrote:
> On Apr 29,  5:07pm, Daniel Shahaf wrote:
> }
> } Pre-existing problem: should the function check that cd succeeded?
> 
> Aside from stderr messing up the display, are there any consequences
> for the completion result if failure is detected rather than ignored?

After looking closer at it, I think it would be better if it would work with non-existing files/dirs instead.

My use case is to make files that are relative to the .git dir relative to $PWD:

    for f in $files; do
      relfiles+=($(relative $gitdir/$f $PWD))
    done

And there it might be that the file does not exist anymore, since it was deleted.
I would still like to get the relative path in that case.
But then the magic of changing the first argument into a dir based on if it is a file will not work anymore obviously.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH] Functions/Misc/relative: use -q with cd

[Bart Schaefer]

On Jun 22, 10:32pm, Daniel Hahler wrote:
}
} After looking closer at it, I think it would be better if it would
} work with non-existing files/dirs instead.

Changing $(cd $1; pwd -r) to ${1:P} would fix that, too, provided that
you also remove the is-a-directory checks on line 7.