Re: patch: zshmisc(1) clarify non-successful exit statuses

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

zeurkous@xxxxxxxx wrote on Tue, Apr 13, 2021 at 20:03:41 +0200:
> "Daniel Shahaf" <d.s@xxxxxxxxxxxxxxxxxx> wrote:
> > Thanks for the patch. Review:
> >
> > zeurkous@xxxxxxxx wrote on Sun, Apr 11, 2021 at 14:15:20 +0200:
> >> #?patch
> >> #
> >
> > What's this header line? Is this a standard format for unidiffs with
> > log messages? Should Functions/VCS_Info/VCS_INFO_patch2subject grow
> > support for it?
> 
> No, it's a "shehuh" (me own convention), to indicate the format (in this
> case: input to patch(1)). Ignore it if you wish. The rest of the '#'
> lines are just comments (meknows patch(1) will relay && discard any
> lines before the actual patch, but IMO it shouldn't, hence the prefix).
> 
> patch(1) will eat all this fine (at least it does on me side).

Every patch(1) implementation in the last few decades behaves this way.

> > zsh's source code is in git. git's interchange format is `[PATCH]' in
> > the subject line, then in the body, everything up to a "---" line is
> > part of the log message, and everything after is not. See
> > git-format-patch(1) for details.
> 
> Thanks, but me doesn't use git (me has me own, very sufficient, ways to
> keep track of things). So me just used 'diff -u'.

You can get svn working copies and tar.gz or zip exports from
https://github.com/zsh-users/zsh/.

> >> # Hope this is useful (it is to me),
> >> #
> >> #         --zeurkous, Sun Apr 11 11:12:21 UTC 2021.
> >> #
> >> --- Doc/Zsh/..v/5.8/exec.yo Mon Dec 4 14:09:35 2017
> >> +++ Doc/Zsh/exec.yo Sun Apr 11 10:42:15 2021
> >> @@ -16,7 +16,10 @@
> >> Otherwise, the shell searches each element of tt($path) for a
> >> directory containing an executable file by that name. If the
> >> search is unsuccessful, the shell prints an error message and returns
> >> -a nonzero exit status.
> >> +127.
> >> +
> >> +If execution fails because of insufficient permissions, or because the
> >> +file is a directory, the shell prints an error message and returns 126.
> >>
> >
> > Does this sentence cover every possible case of returning 126? The
> > condition in the source is «== EACCES || == ENOEXEC».
> 
> Me wishes me had a guarantee. It probably differs from system to system
> (me runs OpenBSD); this is me best shot (for now).

Does that mean you won't be submitting a revised patch?  No problem if
so; just want to make it explicit whether or not revising the patch is
a task that's up for grabs.

> > Moreover, the
> > very next sentence says "the file is not in executable format", and it
> > would be odd to refer to the same condition by different noun phrases in
> > two consecutive sentences.
> 
> In me defense: the whole text is a bit vague, and me's not sure me
> understands the subtleties enough to do a {,partial }rewrite of the
> section.
> 
> > I don't like the newly-added paragraph break. Anyone who stops reading
> > at the end of that paragraph will think the return code is 127, period.
> 
> Why would someone stop reading there...?

People skim.

> > Also, stating the return values before going on to say that if the file
> > isn't a directory then it's exeuted anyway could be confusing, couldn't it?
> 
> In all honesty: to me, the whole text is so unnecessarily unclear that
> me'd apply me usual solution of writing exit statuses in a table:
> 
>  If execution fails: one of the following values is returned.
>  .Pp
>  .Bl -tag -width 126 -compact
>  .It Li 126
>  [...summarize the condition{,s} that cause{,s} it to return that...]
>  .It Li 127
>  [...idem...]
>  .El
> 
> (That's in mdoc(7), not in man(7). Sorry.)
> 
> However, if you folks would desire that: me has no idea how to write
> that in yodl, so mecan't provide a patch.

In this case, a startsitem() would probably work well.  There's plenty
of examples in our tree.

> > Should this part of the manual mention the AUTO_CD option?
> 
> Dunno. It could unnecessarily complicate it.
> 

"Unnecessarily"?  Doesn't AUTO_CD logically belong in that part of the manual?

> > A few paragraph below the value, 127, is mentioned again. Does that
> > sentence need to be updated?
> 
> Me'd say that if the text above would be made sufficiently clear (with
> quotes of both diagnostic messages), that paragraph could be
> significantly curtailed, instead of extended.

That's what I meant.

Daniel