Re: [PATCH v2 1/4] worktree: refactor infer_backlink() to use *strbuf
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Oct 7, 2024, 03:26 UTC
- Message-ID
- <CAPig+cTDK7dk-BNgidQnyTBXi7dqQ1uZ91geerzN4aLtJyHkgw@mail.gmail.com>
- In-Reply-To
- <LyrqJt-X7sHVSKc-97V4-hTsDiIuz8ucEUMDtLCPWAN15k31EBjZnbeuJ2PldcCVvZaPyudvgb3H00T6uZQsI-XON7Ku8jNFjkfRCgrdr8U=@pm.me>
On Sun, Oct 6, 2024 at 10:42 PM Caleb White <cdwhite3@pm.me> wrote:
Show 15 quoted lines
> On Sunday, October 6th, 2024 at 13:16, Eric Sunshine <sunshine@sunshineco.com> wrote:
> > ... this code now becomes more than a little confusing to read. It
> > says "if infer_backlink then signal error", which sounds rather
> > backward and leaves the reader scratching his or her head. ("Why
> > signal error if the function succeeded?"). Hence, infer_backlink()
> > should probably return 1 on success and 0 on failure, which will make
> > this code read more idiomatically:
> >
> > if (!infer_backlink(realdotgit.buf, &backlink)) {
> > ...signal error...
>
> This was my first thought, however, on unix it is fairly standard
> to return 0 if successful and a non-zero int if there's an error.
> I don't mind updating, but I want to follow what makes the most
> sense and would be most expected.I mentioned it because it made my reading hiccup, but I don't feel too strongly about it one way or the other considering that this is an internal function.