zsh-workers
 help / color / mirror / code / Atom feed
From: Daniel Shahaf <d.s@daniel.shahaf.name>
To: zeurkous@volny.cz
Cc: zsh-workers@zsh.org
Subject: Re: patch: zshmisc(1) clarify non-successful exit statuses
Date: Tue, 13 Apr 2021 15:52:36 +0000	[thread overview]
Message-ID: <20210413155236.GR6819@tarpaulin.shahaf.local2> (raw)
In-Reply-To: <20210411141520.4ABA89D5@volny.cz>

Thanks for the patch.  Review:

zeurkous@volny.cz 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?

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.

More below.

> # These patches add, to the zshmisc(1) manual page, clarity about the
> # exit status on exec failure.
> #
> # Me understands that, strictly considered, only Doc/Zsh/exec.yo needs
> # updating; however, as me doesn't have yodl, me updated Doc/zshmisc.1
> # as well.

Thanks, but there's no need to manually update the .1 files; they aren't
in version control.

> # 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».  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.

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.
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?

Should this part of the manual mention the AUTO_CD option?

A few paragraph below the value, 127, is mentioned again.  Does that
sentence need to be updated?

Thanks for the patch!  I realize the review is actually longer than the
patch, but, it's still shorter than the cumulative number of times the
new text will be read.

Cheers,

Daniel

>  If execution fails because the file is not in executable format,
>  and the file is not a directory, it is assumed to be a shell
> 


  reply	other threads:[~2021-04-13 15:53 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-11 12:15 zeurkous
2021-04-13 15:52 ` Daniel Shahaf [this message]
2021-04-13 18:03   ` zeurkous
2021-04-14 11:03     ` Daniel Shahaf
2021-04-14 11:40       ` zeurkous
2021-04-23 12:18         ` revised " zeurkous
2021-04-23 16:07           ` Daniel Shahaf
2021-04-23 16:15             ` Daniel Shahaf
2021-04-23 20:25             ` zeurkous
2021-04-23 20:14           ` Lawrence Velázquez
2021-04-23 20:29             ` zeurkous
2021-04-23 20:39               ` Lawrence Velázquez
2021-04-23 20:57                 ` zeurkous
2021-04-23 20:51           ` Bart Schaefer
2021-04-23 21:03             ` zeurkous
2021-04-23 21:12               ` Bart Schaefer
2021-04-23 21:18                 ` zeurkous
2021-04-23 22:31                   ` Bart Schaefer
2021-04-26 16:25             ` zeurkous
2021-04-29 14:20               ` Daniel Shahaf
2021-05-09 19:04                 ` Lawrence Velázquez
2021-05-31  1:09                   ` Lawrence Velázquez
2021-05-31  4:09                     ` Bart Schaefer
2021-05-31  4:37                       ` Lawrence Velázquez
2021-06-30 17:51                     ` zeurkous
2021-06-30 22:12                       ` Lawrence Velázquez

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210413155236.GR6819@tarpaulin.shahaf.local2 \
    --to=d.s@daniel.shahaf.name \
    --cc=zeurkous@volny.cz \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).