From: Daniel Shahaf <d.s@daniel.shahaf.name>
To: Marlon Richert <marlon.richert@gmail.com>
Cc: Zsh hackers list <zsh-workers@zsh.org>
Subject: Re: [RFC][PATCH] `newuser` prompt theme
Date: Thu, 15 Apr 2021 20:27:17 +0000 [thread overview]
Message-ID: <20210415202717.GA6669@tarpaulin.shahaf.local2> (raw)
In-Reply-To: <CAHLkEDuxrYx_ywG8hLYunz28yy3RMdsk6DBNGDfDjmF9xqpPOQ@mail.gmail.com>
[ 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@daniel.shahaf.name> 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.
next prev parent reply other threads:[~2021-04-15 20:27 UTC|newest]
Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-13 19:41 Marlon
2021-04-13 20:31 ` Roman Perepelitsa
2021-04-13 20:44 ` Bart Schaefer
2021-04-14 5:09 ` Marlon Richert
2021-04-14 5:17 ` Bart Schaefer
2021-04-14 5:45 ` Marlon
2021-04-14 12:05 ` Daniel Shahaf
2021-04-14 13:17 ` Marlon
2021-04-14 14:09 ` Daniel Shahaf
2021-04-15 1:07 ` Bart Schaefer
2021-04-15 3:50 ` Marlon Richert
2021-04-15 20:27 ` Daniel Shahaf [this message]
2021-04-15 23:42 ` Daniel Shahaf
2021-04-15 11:11 ` Mikael Magnusson
2021-04-15 16:44 ` vcs_info's global variables (was: Re: [RFC][PATCH] `newuser` prompt theme) Daniel Shahaf
2021-04-16 16:04 ` [RFC][PATCH] `newuser` prompt theme Marlon
2021-04-16 17:13 ` Daniel Shahaf
2021-04-16 19:19 ` Marlon Richert
2021-04-16 19:30 ` Daniel Shahaf
2021-04-19 16:46 ` Marlon Richert
2021-05-02 17:18 ` Lawrence Velázquez
2021-05-03 2:38 ` Bart Schaefer
2021-05-03 4:11 ` Lawrence Velázquez
2021-05-03 11:38 ` [PATCH] Add customizable `vcs` prompt theme (was Re: [RFC][PATCH] `newuser` prompt theme) Marlon Richert
2021-05-09 18:04 ` Lawrence Velázquez
2021-05-31 1:01 ` Lawrence Velázquez
2021-06-09 12:58 ` Marlon Richert
2021-06-09 13:22 ` Roman Perepelitsa
2021-06-09 18:06 ` Marlon Richert
2021-06-10 9:30 ` Roman Perepelitsa
2021-06-10 13:47 ` Marlon Richert
2021-06-10 14:01 ` Marlon Richert
2021-06-10 17:45 ` Roman Perepelitsa
2021-06-20 22:13 ` Marlon Richert
2021-06-23 17:26 ` Roman Perepelitsa
2022-08-08 10:12 ` Marlon Richert
2022-08-08 10:17 ` Roman Perepelitsa
2021-04-14 14:09 ` sysexits.h codes? (was: " Daniel Shahaf
2021-04-30 19:40 ` Marlon Richert
2021-04-30 21:16 ` Daniel Shahaf
2021-04-30 21:34 ` Bart Schaefer
2021-05-01 15:09 ` Daniel Shahaf
2021-04-30 21:40 ` Bart Schaefer
2021-05-01 13:39 ` Marlon Richert
2021-05-01 14:43 ` Daniel Shahaf
2021-05-03 11:36 ` Marlon Richert
2021-05-03 16:04 ` Daniel Shahaf
2021-05-04 11:13 ` Marlon Richert
2021-05-21 11:38 ` Marlon Richert
2021-04-14 15:30 ` [RFC][PATCH] `newuser` prompt theme Arseny Maslennikov
2021-04-14 18:52 ` Daniel Shahaf
2021-04-14 14:00 ` precmd hooks and $? Bart Schaefer
2021-04-14 15:18 ` docs patches for precmd hooks and $?, and vcs_info Daniel Shahaf
2021-04-15 2:35 ` Bart Schaefer
2021-04-15 16:17 ` Archives render attachments as first-class messages (was: docs patches for precmd hooks and $?, and vcs_info) Daniel Shahaf
2021-04-15 18:54 ` [RFC][PATCH] Reset ZLE hooks when changing prompt themes (was Re: [RFC][PATCH] `newuser` prompt theme) Marlon
2021-04-15 21:34 ` Daniel Shahaf
2021-04-16 22:34 ` Marlon Richert
2021-04-25 16:08 ` Lawrence Velázquez
2021-05-02 17:03 ` Lawrence Velázquez
2021-05-02 17:59 ` Bart Schaefer
2021-05-05 6:10 ` Marlon Richert
2021-05-16 15:27 ` Lawrence Velázquez
2021-05-16 19:31 ` Marlon Richert
2021-05-17 4:19 ` Bart Schaefer
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=20210415202717.GA6669@tarpaulin.shahaf.local2 \
--to=d.s@daniel.shahaf.name \
--cc=marlon.richert@gmail.com \
--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).