From: Jiamu Sun <39@barroit.sh> Date: Mon, 09 Mar 2026 02:06:55 GMT Subject: Re: [PATCH 2/5] help: refactor command autocorrection handling Message-ID: In-Reply-To: On Sun, Mar 08, 2026 at 04:52:54PM -0700, Junio C Hamano wrote: > You snuck in unnecessary "style fixes" to make bunch of "if()return" > into if/else if cascade. Also what was AUTOCORRECT_SHOW is now > returned as AUTOCORR_HINTONLY. There is no explanation on the > reason behind these changes in the proposed log message, and hiding > these small changes in a code movement patch makes reviewing the > series harder than necessary. > > The patch is doing too many things (namely, (1) code movement that > will make it later reusable as a side effect but has no semantic > changes in the current code, plus (2) change in style (like the one > we see here), semantics (possibly the difference in SHOW and > HINTONLY we see here) and features, possibly including the renaming > of AUTOCORRECT_* into AUTOCORR_*.) Let's have "restructure with > code movement and nothing else", followed by "other changes > > I'll stop here, and expect this step to be split into at least two > patches to make it more readable before we can review it again. > > Thanks. Ah, I realized this patch is completely unreadable. You are right, the code movement ended up hiding other changes entirely. Will split it to make each change obvious. -- Jiamu Sun <39@barroit.sh>