RE: [PATCH v12 5/6] fsmonitor: check for compatability before communicating with fsmonitor
Show 39 quoted lines
> -----Original Message-----
> From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Sent: Monday, September 26, 2022 11:24 AM
> To: Eric DeCosta via GitGitGadget <gitgitgadget@gmail.com>
> Cc: git@vger.kernel.org; Jeff Hostetler <git@jeffhostetler.com>; Eric Sunshine
> <sunshine@sunshineco.com>; Torsten Bögershausen <tboegi@web.de>;
> Ramsay Jones <ramsay@ramsayjones.plus.com>; Johannes Schindelin
> <Johannes.Schindelin@gmx.de>; Eric DeCosta <edecosta@mathworks.com>
> Subject: Re: [PATCH v12 5/6] fsmonitor: check for compatability before
> communicating with fsmonitor
>
>
> On Sat, Sep 24 2022, Eric DeCosta via GitGitGadget wrote:
>
> > From: Eric DeCosta <edecosta@mathworks.com> [...] @@ -281,9 +283,11
> @@
> > char *fsm_settings__get_incompatible_msg(const struct repository *r,
> > goto done;
> >
> > case FSMONITOR_REASON_NOSOCKETS:
> > + socket_dir = dirname((char *)fsmonitor_ipc__get_path(r));
> > strbuf_addf(&msg,
> > - _("repository '%s' is incompatible with fsmonitor
> due to lack of Unix sockets"),
> > - r->worktree);
> > + _("socket directory '%s' is incompatible with
> fsmonitor due"
> > + " to lack of Unix sockets support"),
> > + socket_dir);
>
> Could do with less "while at it" here. We are:
>
> * Wrapping the string, making the functional change(s) harder to spot.
> * replacing r->worktree with socket_dir
> * Adding " support" to the end of the string, and replacing "repository" with
> "socket directory"
>
> AFAICT the continuation of the string isn't indented in the way we usually do,
> i.e. to align with the opening ".The string, when properly indented, exceeds an 80 character line length. I'll fix the indentation, but I don't think there's a much better alternative to the wrapping.
The worktree could be in a perfectly fine location whereas the socket_dir may not . Crafting the error message the way I did reflects where the problem is rather than reporting a potentially misleading error about the repository.
-Eric
Eric DeCosta <edecosta@mathworks.com>