Re: [RFC][PATCH] `newuser` prompt theme

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

[ The parent message made both technical points and meta/personnel/communication
points.  In this message I respond only to the former. ]

Marlon Richert wrote on Thu, Apr 15, 2021 at 06:50:47 +0300:
> On Wed, Apr 14, 2021 at 5:09 PM Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx> wrote:
> > > Sure, but what should I call it, then? Just `marlon`? (Seeing as we already have `adam`, `bart`, etc. themes.)
> >
> > That'd work.  A name that describes the theme itself rather than its
> > origin would be even better (if the theme is accepted into zsh.git).
> 
> How about `simple` or `minimal` or something like that?
> 

These wouldn't be my first choices, given prompt_off_setup's contents.

> * You said my prompt theme should not set styles, but what methods
> should I then use to style the VCS part of the prompt?

workers/48577 and workers/48586 give one idea.

> > In any case, what you've implemented is that you re-set a style on the
> > first precmd after every chdir.  I don't think these semantics should be
> > implemented in the first place.
> 
> So, what do you think I should I do instead?

Instead of what?

> How do you think I can improve my code to be less "surprising"?

First, the semantics are surprising in themselves.  There's no obvious
reason why the theme would re-set check-for-staged-changes after every
chdir (and overwrite any user-set value of the style under that context
pattern).

Second, vcs_info_msg_0_ normally neither gets unset nor is used as a flag.

> > Document your code.
> 
> For which sections in particular would you like me to explain my
> motives?

None.  It's not the purpose of comments to explain the author's motives.
${histchars[3]} does not introduce whodunits ;-)

> I try to write my code to be self-documenting. However, as with all
> writing, one easily becomes blond to one's own typos. It would help a
> lot if you could tell me exactly what parts of the code you think need
> comments.

For starters, note that ${(v)_prompt_newuser_formats[(R)start:*]} don't
all undergo the same sequence of expansions, but there's zero hints to
that.  (And while there, compare 48574/0005.)

> > Make defaults overridable.
> 
> Did you read prompt_newuser_help() / the output of `prompt -h
> newuser`? It explains the mechanisms I've prodiv to override the
> defaults.
> 

Some of them, but see check-for-changes for a counter-example (which is
specifically documented as expensive, and has been mentioned in workers/48402
and in discussions about your proposed default zshrc in workers/48544).

Speaking of which, the help output is wrong because it instructs to set
styles for context patterns that aren't specific to that prompt theme.

> > (e.g., someone explicitly setting that style
> > to a false value for that same context, or for the context «:vcs_info*»,
> > which is less specific than the context you use and thus would be shadowed).
> 
> This theme is intended for persons who don't want to manually
> configure their VCS info separately from their prompt.

The point remains that the «:vcs_info:*» context patterns zstyle space
is global settings.  As it stands, the prompt theme will affect all uses
of vcs_info whilst the prompt theme is in effect, and even affect uses
of vcs_info after another prompt theme is loaded.

> My theme is also setting [R]PS(1|2|4). Do you see that, too, as
> overriding the user's settings?

No.  Per prompt_default_setup, for a prompt theme to set PS1 is as for
«echo hello world» to print «hello world
».

> If the user wants to configure those manually, then they can choose
> not to use this theme.

For PS1, sure, but PS1 isn't to a prompt theme as the «:vcs_info:*»
zstyle space is to a prompt theme.

> > > > That appears to be NIH.
> > >
> > > Sorry, but what does NIH stand for? (I’m guessing you don’t mean the National Institutes of Health.)
> >
> > Not Invented Here syndrome; cf. EEE :P
> 
> What exactly makes you think I have Not Invented Here syndrome? I am
> reusing vcs_info instead of rolling my own Git inspection code, am I
> not?

Here's the context again:

> > > > Your theme does _nothing_ with the 'unstaged' style other than
> > > > pass it through verbatim.  That appears to be NIH.

> > > > Why shouldn't the theme just advise people to set the vcs_info
> > > > directly?
> > >
> > > Because it’s a theme? If people wanted to style their vcs_info
> > > directly, then why would they use a theme?
> >
> > The point is, you're making people learn a different syntax for
> > identical functionality for zero benefit.
> 
> "Zero" benefit? vcs_info's documentation is not easy to read.

Then send a bug report.

> I am creating a simpler abstraction layer on top of a rather tricky
> API.

The *one* thing which your styles provide is the equivalent of «zstyle …
formats $a$b; zstyle … actionformats $a$c»; there's no other case where
«prompt_newuser_format» is called with ≥3 arguments.  For unstagedstr,
you provide just a rename; and for cont:left and cont:indent, which are
two knobs that are concatenated to each other and aren't used in any
other way, you just _add_ complexity.

No comment on whether or not vcs_info's API is "tricky".

> If I thought there was "zero" benefit in what I was doing, then I
> wouldn't have programmed it that way, would?

Presumably you _thought_ there was benefit in that approach — but your
reviewers don't happen to agree.  Don't take that personally.