[-- Attachment #1: Type: text/plain, Size: 563 bytes --] I’m (slowly) reading through zshexpn and playing with things to learn. This does not work: > pedz@MysticSlate my-play-dir % echo /this/is/a/../../path > /this/is/a/../../path > pedz@MysticSlate my-play-dir % echo !$:P > zsh: illegal modifier: P even if the file specified exists but this does: > pedz@MysticSlate my-play-dir % echo /this/is/a/../../path > /this/is/a/../../path > pedz@MysticSlate my-play-dir % echo !$:A > echo /this/path > /this/path Is this a documentation bug, code bug, or my misunderstanding? Thank you, pedz [-- Attachment #2: smime.p7s --] [-- Type: application/pkcs7-signature, Size: 2966 bytes --]
On Sun, 2019-11-17 at 12:42 -0600, Perry Smith wrote: > I’m (slowly) reading through zshexpn and playing with things to > learn. This does not work: > > > > > pedz@MysticSlate my-play-dir % echo /this/is/a/../../path > > /this/is/a/../../path > > pedz@MysticSlate my-play-dir % echo !$:P > > zsh: illegal modifier: P > even if the file specified exists I think it just got missed out of the history modifiers, which are handled in a different place from the modifiers in the case of glob qualifiers and vaariables. pws diff --git a/Src/hist.c b/Src/hist.c index fd5606dc3..e47be8e15 100644 --- a/Src/hist.c +++ b/Src/hist.c @@ -920,6 +920,16 @@ histsubchar(int c) case 'u': sline = casemodify(sline, CASMOD_UPPER); break; + case 'P': + if (*sline != '/') { + char *here = zgetcwd(); + if (here[strlen(here)-1] != '/') + sline = zhtricat(metafy(here, -1, META_HEAPDUP), "/", sline); + else + sline = dyncat(here, sline); + } + sline = xsymlink(sline, 1); + break; default: herrflush(); zerr("illegal modifier: %c", c);
Peter Stephenson wrote on Mon, 18 Nov 2019 09:47 +00:00: > On Sun, 2019-11-17 at 12:42 -0600, Perry Smith wrote: > > I’m (slowly) reading through zshexpn and playing with things to > > learn. This does not work: > > > > > > > > pedz@MysticSlate my-play-dir % echo /this/is/a/../../path > > > /this/is/a/../../path > > > pedz@MysticSlate my-play-dir % echo !$:P > > > zsh: illegal modifier: P > > even if the file specified exists > > I think it just got missed out of the history modifiers, which are > handled in a different place from the modifiers in the case of glob > qualifiers and vaariables. Thanks for fixing this. Test added in 030440d5b7bfd2968138836e962f1ef61ea0bae8. > +++ b/Src/hist.c > @@ -920,6 +920,16 @@ histsubchar(int c) > + case 'P': > + if (*sline != '/') { > + char *here = zgetcwd(); > + if (here[strlen(here)-1] != '/') Can «zgetcwd()» return an empty string? If that's possible, the condition will be undefined behaviour («""[-1]»). > + sline = zhtricat(metafy(here, -1, META_HEAPDUP), "/", sline); > + else > + sline = dyncat(here, sline); > + } > + sline = xsymlink(sline, 1); > + break; Cheers, Daniel
On Tue, 2019-11-19 at 18:00 +0000, Daniel Shahaf wrote:
> Can «zgetcwd()» return an empty string? If that's possible, the
> condition will be undefined behaviour («""[-1]»).
Not in a sane world, but there are cases where we can't work out the
current directory. I think we return "." then, in fact. But a bit of
safety probably wouldn't hurt --- applying [-1] to functions arguably
isn't top class style.
pws
Peter Stephenson wrote on Tue, 19 Nov 2019 18:21 +00:00: > On Tue, 2019-11-19 at 18:00 +0000, Daniel Shahaf wrote: > > Can «zgetcwd()» return an empty string? If that's possible, the > > condition will be undefined behaviour («""[-1]»). > > Not in a sane world, but there are cases where we can't work out the > current directory. I think we return "." then, in fact. But a bit of > safety probably wouldn't hurt --- applying [-1] to functions arguably > isn't top class style. How about this, then? diff --git a/Src/compat.c b/Src/compat.c index 7131d91a4..02b66a780 100644 --- a/Src/compat.c +++ b/Src/compat.c @@ -519,7 +519,7 @@ zgetcwd(void) #endif /* HAVE_GETCWD */ if (!ret) ret = unmeta(pwd); - if (!ret) + if (!ret || *ret == '\0') ret = dupstring("."); return ret; } Cheers, Daniel (let's continue the discussion on -workers@)