* [BUG] zf_ln complains about the wrong argument @ 2021-08-22 13:45 Marlon Richert 2021-08-22 15:35 ` Mikael Magnusson 2021-08-22 20:24 ` Peter Stephenson 0 siblings, 2 replies; 7+ messages in thread From: Marlon Richert @ 2021-08-22 13:45 UTC (permalink / raw) To: Zsh hackers list When the second argument to zf_ln is an empty string, zf_ln mistakenly reports that the first argument is a non-existing file or dir: % zmodload zsh/files % touch foo % zf_ln foo '' zf_ln: foo: no such file or directory ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [BUG] zf_ln complains about the wrong argument 2021-08-22 13:45 [BUG] zf_ln complains about the wrong argument Marlon Richert @ 2021-08-22 15:35 ` Mikael Magnusson 2021-08-22 19:27 ` Marlon Richert 2021-08-22 20:24 ` Peter Stephenson 1 sibling, 1 reply; 7+ messages in thread From: Mikael Magnusson @ 2021-08-22 15:35 UTC (permalink / raw) To: Marlon Richert; +Cc: Zsh hackers list On 8/22/21, Marlon Richert <marlon.richert@gmail.com> wrote: > When the second argument to zf_ln is an empty string, zf_ln mistakenly > reports that the first argument is a non-existing file or dir: > > % zmodload zsh/files > % touch foo > % zf_ln foo '' > zf_ln: foo: no such file or directory As do zf_mv, gnu ln, and gnu mv. -- Mikael Magnusson ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [BUG] zf_ln complains about the wrong argument 2021-08-22 15:35 ` Mikael Magnusson @ 2021-08-22 19:27 ` Marlon Richert 0 siblings, 0 replies; 7+ messages in thread From: Marlon Richert @ 2021-08-22 19:27 UTC (permalink / raw) To: Mikael Magnusson; +Cc: Zsh hackers list On Sun, Aug 22, 2021 at 6:35 PM Mikael Magnusson <mikachu@gmail.com> wrote: > > On 8/22/21, Marlon Richert <marlon.richert@gmail.com> wrote: > > When the second argument to zf_ln is an empty string, zf_ln mistakenly > > reports that the first argument is a non-existing file or dir: > > > > % zmodload zsh/files > > % touch foo > > % zf_ln foo '' > > zf_ln: foo: no such file or directory > > As do zf_mv, gnu ln, and gnu mv. Well, two wrongs don't make a right, don't you think? Just because Gnu prints confusing error messages, doesn't mean Zsh can't do better. Plus, BSD ln does manage to complain about the right file: % ln foo '' ln: : No such file or directory Also, I discovered this happens, too, when trying to create a symlink in a dir that doesn't exist: % zf_ln foo bar/baz zf_ln: foo: no such file or directory BSD ln again gives a better error message (although still not 100% accurate): % ln foo bar/baz ln: bar/baz: No such file or directory ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [BUG] zf_ln complains about the wrong argument 2021-08-22 13:45 [BUG] zf_ln complains about the wrong argument Marlon Richert 2021-08-22 15:35 ` Mikael Magnusson @ 2021-08-22 20:24 ` Peter Stephenson 2021-08-22 21:05 ` Daniel Shahaf 1 sibling, 1 reply; 7+ messages in thread From: Peter Stephenson @ 2021-08-22 20:24 UTC (permalink / raw) To: zsh-workers On Sun, 2021-08-22 at 16:45 +0300, Marlon Richert wrote: > When the second argument to zf_ln is an empty string, zf_ln mistakenly > reports that the first argument is a non-existing file or dir: > > % zmodload zsh/files > % touch foo > % zf_ln foo '' > zf_ln: foo: no such file or directory Is this good enough to detect cases like this? An extra stat in the error case should be neither here nor there. Also seems a good idea to turn a null string into a couple of quotes. I thought of modifying the error message handler but this has too many knock on effects. pws diff --git a/Src/Modules/files.c b/Src/Modules/files.c index a1d6f6bf2..d4ec93624 100644 --- a/Src/Modules/files.c +++ b/Src/Modules/files.c @@ -346,7 +346,16 @@ domove(char *nam, MoveFunc movefn, char *p, char *q, int flags) unlink(qbuf); } if(movefn(pbuf, qbuf)) { - zwarnnam(nam, "%s: %e", p, errno); + int ferrno = errno; + char *errfile = p; + if (ferrno == ENOENT && !lstat(pbuf, &st)) { + /* p *does* exist, so error is in q */ + errfile = q; + } + if (*errfile) + zwarnnam(nam, "%s: %e", errfile, ferrno); + else + zwarnnam(nam, "`': %e", ferrno); zsfree(pbuf); return 1; } ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [BUG] zf_ln complains about the wrong argument 2021-08-22 20:24 ` Peter Stephenson @ 2021-08-22 21:05 ` Daniel Shahaf 2021-08-23 8:33 ` Peter Stephenson 0 siblings, 1 reply; 7+ messages in thread From: Daniel Shahaf @ 2021-08-22 21:05 UTC (permalink / raw) To: zsh-workers Peter Stephenson wrote on Sun, 22 Aug 2021 20:24 +00:00: > Also seems a good idea to turn a null string into a couple of quotes. I > thought of modifying the error message handler but this has too many > knock on effects. What "knock on effects"? Extending zwarning() with a %q format code that takes a «char *» and outputs it quoted seems like a good idea and shouldn't break anything. Will handle filenames with spaces, too. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [BUG] zf_ln complains about the wrong argument 2021-08-22 21:05 ` Daniel Shahaf @ 2021-08-23 8:33 ` Peter Stephenson 2021-08-24 13:04 ` Daniel Shahaf 0 siblings, 1 reply; 7+ messages in thread From: Peter Stephenson @ 2021-08-23 8:33 UTC (permalink / raw) To: zsh-workers > On 22 August 2021 at 22:05 Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > Peter Stephenson wrote on Sun, 22 Aug 2021 20:24 +00:00: > > Also seems a good idea to turn a null string into a couple of quotes. I > > thought of modifying the error message handler but this has too many > > knock on effects. > > What "knock on effects"? Extending zwarning() with a %q format code > that takes a «char *» and outputs it quoted seems like a good idea and > shouldn't break anything. Will handle filenames with spaces, too. This is certainly all stuff that's accumulated rather than been planned over the years, but having a harder look I think it's mostly not too bad the way it is. I think the basics are currently as follows. %s as implemented in errors and warnings is already not a straight printf %s: it uses nicezputs() to emphasise visibility rather than quoting, i.e. it's there to make it easy to see what the error is about, rather than easy to copy back into the shell. Given the error is about something that would usually be either in a script or on the command line, this doesn't seem a bad way of doing it, on reflection. As far as quoting is concerned, the state of the art is to use `%s' (i.e. with those literal quotes) if it seems necessary to draw attention to the argument, i.e. it could get lost otherwise. There are a lot of cases where we don't do this, however, but they can easily be changed. The combination of this and nicezputs() means you can always see what's there, unless there are very subtle cases I've missed. So I think the consistent way of handling a case like this is to turn %s into `%s'. That both makes it visible what's going on and makes the output consistent with similar cases elsewhere. Does that seem reasonable? I can't think of a strong argument for a more widespread change given it's sort of working and even sort of potentially consistent. pws ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [BUG] zf_ln complains about the wrong argument 2021-08-23 8:33 ` Peter Stephenson @ 2021-08-24 13:04 ` Daniel Shahaf 0 siblings, 0 replies; 7+ messages in thread From: Daniel Shahaf @ 2021-08-24 13:04 UTC (permalink / raw) To: Peter Stephenson; +Cc: zsh-workers Peter Stephenson wrote on Mon, Aug 23, 2021 at 09:33:21 +0100: > > > On 22 August 2021 at 22:05 Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > Peter Stephenson wrote on Sun, 22 Aug 2021 20:24 +00:00: > > > Also seems a good idea to turn a null string into a couple of quotes. I > > > thought of modifying the error message handler but this has too many > > > knock on effects. > > > > What "knock on effects"? Extending zwarning() with a %q format code > > that takes a «char *» and outputs it quoted seems like a good idea and > > shouldn't break anything. Will handle filenames with spaces, too. > > This is certainly all stuff that's accumulated rather than been planned > over the years, but having a harder look I think it's mostly not too bad > the way it is. I think the basics are currently as follows. > > %s as implemented in errors and warnings is already not a straight > printf %s: it uses nicezputs() to emphasise visibility rather than quoting, > i.e. it's there to make it easy to see what the error is about, rather than > easy to copy back into the shell. Given the error is about something that > would usually be either in a script or on the command line, this doesn't > seem a bad way of doing it, on reflection. > Well, sure, %s doesn't have to use the an escaping algorithm that's inputtable back into the shell, but it'd still be nice if the escaping were clear and unambiguous. As it stands, printing «%s» doesn't escape or quote empty strings and spaces. > As far as quoting is concerned, the state of the art is to use `%s' (i.e. > with those literal quotes) if it seems necessary to draw attention to the > argument, i.e. it could get lost otherwise. There are a lot of cases > where we don't do this, however, but they can easily be changed. The > combination of this and nicezputs() means you can always see what's > there, unless there are very subtle cases I've missed. If the argument to %s contains literal single quotes then this is ambiguous. > So I think the consistent way of handling a case like this is to turn > %s into `%s'. That both makes it visible what's going on and makes > the output consistent with similar cases elsewhere. > > Does that seem reasonable? +1 > I can't think of a strong argument for a more widespread change given > it's sort of working and even sort of potentially consistent. Well, if there were a %q that did unambiguous escaping/quoting, it'd be harder to accidentally write zwarning("foo: %s: bar") in new code, forgetting to quote. And of course, filenames with single quotes would be output more unambiguously :) Cheers, Daniel ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-08-24 13:06 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-22 13:45 [BUG] zf_ln complains about the wrong argument Marlon Richert 2021-08-22 15:35 ` Mikael Magnusson 2021-08-22 19:27 ` Marlon Richert 2021-08-22 20:24 ` Peter Stephenson 2021-08-22 21:05 ` Daniel Shahaf 2021-08-23 8:33 ` Peter Stephenson 2021-08-24 13:04 ` Daniel Shahaf
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).