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

[Marlon Richert <marlon.richert@xxxxxxxxx>]

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?

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

I don't know your background, but where I'm from, saying "please"
means being polite. I don't think I've given you any reason to think
otherwise. Have I?

However, if you do think I'm doing something wrong, then please make
your criticism more constructive.
* You imply I was being rude, but what in my sentence made you think
so? Without knowing that, I cannot correct the way I write.
* You said my prompt theme should not set styles, but what methods
should I then use to style the VCS part of the prompt? I cannot know
how you want me to change my code, if you don't tell me.

I would appreciate it if you could try to keep your code review
comments actionable and on topic, please. Thank you.

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

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

I would appreciate it if you could use less loaded words than
"surprise" and "mislead". I am not trying to surprise my
readers/users, let alone "mislead" them. Please try to stick to the
facts and write actionable comments. How do you think I can improve my
code to be less "surprising"?

> Document your code.

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 which sections in particular would you like me to
explain my motives?

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

> Don't trample user settings

It's one thing to tell me not to override a user settings, but quite
another to tell me that I "trample" them.

> (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. My theme is
also setting [R]PS(1|2|4). Do you see that, too, as overriding the
user's settings? If the user wants to configure those manually, then
they can choose not to use this theme.

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

I would appreciate it if you could refrain from insulting my
intellect. I am new as an open source contributor in general and to
Zsh in particular. So, please bear in mind that I might not be used to
the "ground rules" under which you operate.

I have, however, written code for nearly 30 years and worked
professionally in the software industry for over 20 years. Yet, I have
never seen so much unconstructive criticism in a code review as I've
now read in your email. I would appreciate it if you could adjust your
tone of voice, please, and show a bit more respect for your fellow
programmer.

> > > 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? To quote one famous comedic duo from New Zealand: "Be more
constructive with your feedback, please." Also, I have no idea what
EEE stands for. Should I look it up in the jargon file? You're not
making it easy to understand you.

> > > 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. I am
creating a simpler abstraction layer on top of a rather tricky API. If
I thought there was "zero" benefit in what I was doing, then I
wouldn't have programmed it that way, would?

You said it yourself: "Make defaults overridable." That's what I'm
trying to do here. I am open to suggestions for improvements, but
simply telling me that it has "zero benefit" and leaving it at that is
not being helpful.

Overall, I have to say I'm quite disappointed with your tone of voice
in this email. Perhaps you were having a bad day? I'm hoping that we
can be more pleasant to each other again next time.

Kind regards,
—Marlon