Re: [PATCH] Add customizable `vcs` prompt theme (was Re: [RFC][PATCH] `newuser` prompt theme)

[Roman Perepelitsa <roman.perepelitsa@xxxxxxxxx>]

On Wed, Jun 9, 2021 at 8:06 PM Marlon Richert <marlon.richert@xxxxxxxxx> wrote:
>
> Thanks for the review! Here's a new version of the patch that
> hopefully addresses the concerns you raised.

There is //'%%'/% in a few places. What's the logic behind it?

I think invoking prompt_vcs_setup twice in the same shell will produce
an error. I haven't tried it though.

prompt_vcs_chpwd always prints at least one line to the TTY regardless
of configuration options. Is this intended?

Why is prompt_vcs_precmd checking whether prompt_vcs_fd is a readable
fd? How can it not be?

There is a comment in prompt_vcs_precmd that says it's going to kill a
process while in fact it closes the file descriptor. That may
*eventually* kill the process when it decides to print something
(assuming it hasn't blocked SIGPIPE or is checking for write errors).

What is the purpose of the "Info is not for current dir" check? Is
this to account for the possibility of `cd` from a zle widget? If so,
perhaps handle this by hooking chpwd?

prompt_vcs_fd-widget has a check for [[ -n $1 ]]. When is it false?

Roman.