From: Eric DeCosta Date: Tue, 27 Sep 2022 01:25:35 GMT Subject: RE: [PATCH v12 5/6] fsmonitor: check for compatability before communicating with fsmonitor Message-ID: In-Reply-To: <220926.86v8payx6p.gmgdl@evledraar.gmail.com> > -----Original Message----- > From: Ævar Arnfjörð Bjarmason > Sent: Monday, September 26, 2022 11:24 AM > To: Eric DeCosta via GitGitGadget > Cc: git@vger.kernel.org; Jeff Hostetler ; Eric Sunshine > ; Torsten Bögershausen ; > Ramsay Jones ; Johannes Schindelin > ; Eric DeCosta > 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 [...] @@ -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