zsh-workers
 help / color / mirror / code / Atom feed
From: Daniel Shahaf <d.s@daniel.shahaf.name>
To: zsh-workers@zsh.org
Subject: Re: zsh coding style (was about a limit patch review)
Date: Tue, 1 Dec 2020 16:47:42 +0000	[thread overview]
Message-ID: <20201201164742.GA24476@tarpaulin.shahaf.local2> (raw)
In-Reply-To: <20201128065201.54ngcgcyklyfggm6@chazelas.org>

Stephane Chazelas wrote on Sat, Nov 28, 2020 at 06:52:01 +0000:
> Believe it or not, I had "(boolean)" and a "Returns:..."
> initially, and took them out before submitting as they would
> have been out of character among the other functions in that
> file (and the rest of the code from a cursory look) which are
> rather minimalist and avoid stating the obvious.

Every docstring should state the permitted values of each parameter and the
possible return values.  Yes, some existing functions fall short of this
standard, but there's no reason not to hold _new_ functions to it.  That
doesn't pay off existing technical debt, but it avoids accruing further debt.

> This function description is already by far the least dry and
> probably the most RY in that file. To the point that I'm
> wondering if you're pulling my leg for being overly wordy here.

No, I'm not.

> About the _p suffix, the only usage of it in the code (beside
> has_p) I could find for it was 
> save_params(Estate state, Wordcode pc, LinkList *restore_p, LinkList *remove_p)
> in exec.c, but that "p" looks like it's for "pointer" (as in
> return value, though could also be "param"), not "predicate".
> I've never heard of _p for predicate before, but then again I've
> not done much C development lately.
> 

I hear it's more common in lisp circles.  It's a form of Hungarian notation.

> I'm all for having a coding style. Even a template for function
> header could be useful. That would probably help make the
> code more legible (though in general, I do find zsh's code
> pretty legible, orders of magnitude more so than ksh93's for
> instance)
> 
> I did read Etc/zsh-development-guide before submitting my first
> patch. The relevant bit there is:
> 
> } * Function declarations must look like this:
> } 
> }   /**/
> }   int
> }   foo(char *s, char **p)
> }   {
> }       function body
> }   }
> } 
> }   There must be an empty line, a line with "/**/", a line with the
> }   type of the function, and finally the name of the function with typed
> }   arguments.  These lines must not be indented.  The script generating
> }   function prototypes and the ansi2knr program depend on this format.
> 
> Maybe that document could be updated to say more precisely how
> new code should look like if we're to enforce a new coding
> style.

I wasn't trying to enforce a new coding style.  I was simply reviewing the
docstring to the same standard I've reviewed every other docstring I've ever
reviewed, either on this list or elsewhere.

Cheers,

Daniel


  reply	other threads:[~2020-12-01 16:48 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-23 21:49 [PATCHv1] [long] improvements to limit/ulimit API and doc Stephane Chazelas
2020-11-25  0:35 ` Daniel Shahaf
2020-11-25  6:44   ` Stephane Chazelas
2020-11-27 17:16     ` Daniel Shahaf
2020-11-26  6:57   ` [PATCHv2 1/2] [long] improvements to limit/ulimit API and doc ((un)limit in csh emulation) Stephane Chazelas
2020-11-25 23:43 ` [PATCHv1] [long] improvements to limit/ulimit API and doc Oliver Kiddle
2020-11-26 20:14   ` [PATCH] ulimit option completions using ulimit -a output Stephane Chazelas
2020-11-27  7:13     ` Stephane Chazelas
2020-11-27  8:15       ` Felipe Contreras
2020-11-27 12:19       ` Oliver Kiddle
2021-03-27 21:25         ` Lawrence Velázquez
2021-04-03 14:57           ` Lawrence Velázquez
2021-04-10 20:11             ` Lawrence Velázquez
2021-04-13 14:35     ` Daniel Shahaf
2021-05-09 20:37       ` Lawrence Velázquez
2021-05-11 19:05         ` Stephane Chazelas
2020-11-26 20:58   ` [PATCHv2 2/2] [long] improvements to limit/ulimit API and doc (the rest) Stephane Chazelas
2020-11-27 16:39     ` Daniel Shahaf
2020-11-27 20:13       ` Stephane Chazelas
2020-11-27 20:36         ` Daniel Shahaf
2020-11-28  6:52           ` zsh coding style (was about a limit patch review) Stephane Chazelas
2020-12-01 16:47             ` Daniel Shahaf [this message]
2020-11-28  8:16         ` [PATCHv3 2/2] [long] improvements to limit/ulimit API and doc (the rest) Stephane Chazelas
2021-03-27 21:21           ` Lawrence Velázquez
2021-03-31 18:06             ` Stephane Chazelas
2020-11-26 11:19 ` [PATCHv1] [long] improvements to limit/ulimit API and doc Jun T
2020-11-26 13:55   ` Stephane Chazelas
2020-11-26 15:22     ` Jun. T
2020-11-26 17:23       ` Stephane Chazelas
2020-11-27 18:24         ` Jun. T
2020-11-27 18:34           ` Daniel Shahaf
2020-11-27 20:46           ` Stephane Chazelas

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=20201201164742.GA24476@tarpaulin.shahaf.local2 \
    --to=d.s@daniel.shahaf.name \
    --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).