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

[Marlon <marlon.richert@xxxxxxxxx>]

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


>> @@ -0,0 +1,197 @@
>> +readonly -ga sysexits=(
>> +    USAGE
> ⋮
>> +    CONFIG
> 
> Two interrelated scripts on zsh.org hardcode these constants too.
> I wonder if we should provide these constants in a standard autoloaded
> function or preset variable.

A preset variable would be great. In C code, you can get the names and values from `sysexits.h`, which is available in both GNU and BSD.


>> +  emulate -L zsh
>> +
>> +  psvar[1]=
>> +  case $exitstatus in
>> +    <128-> )
>> +      psvar[1]="SIG$signals[exitstatus-127] "
>> +      ;|
>> +    <64-78> )
>> +      psvar[1]="EX_$sysexits[exitstatus-63] "
> 
> Nice, I might adopt this.  Or, come to think of it, I might teach
> PRINT_EXIT_VALUE to do this ☺

It would be great if, besides printing the name, it could also add a short description of what the exit code actually means. Something like “SIGPIPE (140): write on a pipe with no reader”.


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


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


> Also, there are literally zero comments in the code.  That makes it
> rather unmaintainable by anyone other than you — and I wonder if I
> shouldn't say "other than present-you", since future-you may not fare
> any better than present-me at understanding the code without comments.

Sure, I will add those.


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


> That appears to be NIH.  

Sorry, but what does NIH stand for? (I’m guessing you don’t mean the National Institutes of Health.)


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