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

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

Marlon wrote on Wed, Apr 14, 2021 at 16:17:02 +0300:
> On 14 Apr 2021, at 15:05, Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx> wrote:
> > 
> >> new file mode 100644
> >> +++ b/Functions/Prompts/prompt_newuser_setup
> > 
> > At the risk of bikeshedding, I don't think that's a good name.  A name
> > should describe what a thing *is*, not what its intended use is; and I'm
> > not particularly fond of the implication that new users should be wary
> > of trying _other_ themes.  Besides, what if someone else were to come
> > along and post an alternative theme aimed at new users?
> 
> 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).

> >> +      ;|
> >> +    <1-> )
> >> +      psvar[1]+="($exitstatus)"
> >> +  esac
> >> +
> >> +  if ! [[ -v vcs_info_msg_0_ ]]; then
> >> +    zstyle ':vcs_info:*' check-for-staged-changes yes
> > 
> > First, no other prompt theme sets styles, so I'm not sure prompt themes
> > should be doing that.
> 
> Please show me another way to theme the VCS part of the prompt and I will use that.

You seem to have checked your manners in at the door.

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.

> > Second, even if a prompt were to set styles, doing so in a precmd and
> > behind what _looks_ like a "first run" condition but is actually a "we
> > just changed directory" condition isn't exactly best practice.
> 
> What would be a better practice?

Don't surprise/mislead code readers.

Don't surprise/mislead users.

Document your code.

Make defaults overridable.

Don't trample user settings (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).

These are all such ground rules that I'm surprised I have to spell them out.

> >> +prompt_newuser_setup() {
> >> +  prompt_opts=( cr percent sp )
> >> +
> >> +  zstyle -e ':vcs_info:*' formats '
> >> +    reply=( "%u%c$( prompt_newuser_format start branch repo )" )
> >> +  '
> >> +  zstyle -e ':vcs_info:*' actionformats '
> >> +    reply=( "%u%c$( prompt_newuser_format start action repo )" )
> >> +  '
> >> +  zstyle -e ':vcs_info:*' stagedstr '
> >> +    reply=( "$( prompt_newuser_format start staged )" )
> >> +  '
> >> +  zstyle -e ':vcs_info:*' unstagedstr '
> >> +    reply=( "$( prompt_newuser_format start unstaged )" )
> >> +  '
> > 
> > Your theme does _nothing_ with the 'unstaged' style other than
> > pass it through verbatim.  
> 
> After setting it to '%B%F{red}*%b%f’, yes.
> 

Hmm.  That's a good question: how _should_ a theme set an overrideable
default for stagedstr?

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

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

> Conversely, VCS info is the number one thing that new users want to
> add to their prompt. I don’t think it’s unreasonable to expect that to
> be included out of the box.  Nearly all of 3rd-party prompt themes in
> the wild include at least Git info.

The question remains whether or not for a prompt theme to set vcs_info
styles does or doesn't violate the principle of least surprise.

> > The promptinit changes are independent of the new theme.  They should be
> > a separate patch and could conceivably be applied separately.
> 
> Sure, I’ll start a separate patch thread for those.

Thanks.

>

Daniel