From: Jeff Hostetler Date: Wed, 02 Mar 2022 21:09:28 GMT Subject: Re: [PATCH 05/23] fsmonitor-settings: bare repos are incompatible with FSMonitor Message-ID: <1fe07a1b-df7f-5230-5f49-4cb454c3df9f@jeffhostetler.com> In-Reply-To: <220225.86ee3q7k2s.gmgdl@evledraar.gmail.com> On 2/25/22 3:42 PM, Ævar Arnfjörð Bjarmason wrote: > > On Tue, Feb 15 2022, Jeff Hostetler via GitGitGadget wrote: > >> +static void create_reason_message(struct repository *r, >> + struct strbuf *buf_reason) >> +{ >> + struct fsmonitor_settings *s = r->settings.fsmonitor; >> + >> + switch (s->reason) { >> + case FSMONITOR_REASON_ZERO: >> + return; >> + >> + case FSMONITOR_REASON_BARE: >> + strbuf_addstr(buf_reason, >> + _("bare repos are incompatible with fsmonitor")); >> + return; >> + >> + default: >> + BUG("Unhandled case in create_reason_message '%d'", s->reason); >> + } >> +} >> + >> +enum fsmonitor_reason fsm_settings__get_reason(struct repository *r, >> + struct strbuf *buf_reason) >> +{ >> + lookup_fsmonitor_settings(r); >> + >> + strbuf_reset(buf_reason); >> + if (r->settings.fsmonitor->mode == FSMONITOR_MODE_INCOMPATIBLE) >> + create_reason_message(r, buf_reason); >> + >> + return r->settings.fsmonitor->reason; >> +} > > This API (just looking at one small bit discussed because related bits > conflict with another series) seems to require a lot of ceremony just to > get a const char * error. My thought at the time was that the __get_reason() code might want to format a message and include details from the repo (such as the working directory root) or the kind of objection (such as a remote SMB mount). So I had it take a strbuf rather than a "const char *" return value. But so far (in the successive commits) all of the reason messages have been constant and I think I'm OK with having the non-specific messages. So yes, I could simplify it. Thanks Jeff