Re: [PATCH 3/5] checkout --to: no auto-detach if the ref is already checked out

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Duy Nguyen <pclouds@xxxxxxxxx> writes:

> On Thu, Jul 24, 2014 at 4:16 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>>> +     if (strbuf_read_file(&sb, path.buf, 0) <= 0 ||
>>> +         !skip_prefix(sb.buf, "ref:", &start))
>>> +             goto done;
>>>       while (isspace(*start))
>>>               start++;
>>>       end = start;
>>>       while (*end && !isspace(*end))
>>>               end++;
>>
>> Not new in this round of update, and may not even be an issue, but:
>>
>>  - Earlier, the code returned early on a negative return value from
>>    read-file (i.e., an error), but this round it also does so for
>>    zero.  Intended?
>
> Yes. But it does not make any difference. strbuf_read_file returns
> sb.len, if it's empty, the next skip_prefix would fail anyway.

Yes but changing < 0 to <= 0 is inconsistent with that; I would
understand if you changed it to <= 4, which would be consistent with
the reasoning, though.

>> The code in the patch, which is something in between, does not make
>> much sense to me.
>
> I think more about "echo abc > $this_file" where the echo command may
> output '\r\n' on Windows (wild guess though, I don't use Windows
> much). I think I'm going with _rtrim.

To expect 'echo' into the file is to expect and encourage that
people muck with the internal implementation details by hand, which
we do not generally do for things inside .git [*1*].

If we consider the contents of $this_file not an implementation
detail but a part of the published API (i.e. "writing this string
into the file is a valid way to make Git do this"), rtrim would at
least be consistent with how the existing code deals with symrefs,
so I wouldn't say "does not make much sense" if you are going in
that direction ;-)


[Footnote]

*1* ... except for .git/config, to which we say "It's a simple text
file; don't be afraied to edit it away!".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]