Re: [PATCH 2/2] Documentation/git-log: mention that line-log regex must match in starting revision
- From
Derrick Stolee <stolee@gmail.com>
- Date
- Dec 18, 2019, 10:55 UTC
- Message-ID
- <d8e753ec-2835-331f-99de-35a4a5369b48@gmail.com>
- In-Reply-To
- <147443E8-EA04-45D2-B3BC-D879A5FFECDC@gmail.com>
On 12/17/2019 10:28 PM, Philippe Blain wrote:
Show 6 quoted lines
> >> Le 17 déc. 2019 à 13:16, Junio C Hamano <gitster@pobox.com> a écrit : >> Even when you specify <start> or <end> as a line number, they must >> exist in the starting revision or it would be a fatal error. If we >> are clarifying with this patch for completeness, I think we should >> also mention it together.
...
> git log -L 300,2000000085:Documentation/git-log.txt > errors out: > fatal: file Documentation/git-log.txt has only 239 lines
This case seems important enough to include what it means to "match".
Specifically, the range must match at least one line in the file.
Show 5 quoted lines
> But > git log -L 300,-2000000085:Documentation/git-log.txt > does not error out and shows the history for the whole file. Also, > git log -L 1,-2000000085:Documentation/git-log.txt > does not error out and gives the history for the first line.
Negative numbers in the ranges are a bit strange to include, and the large magnitude you include here seems unnecessary for the test. However, it appears that we do store signed values in the line-log machinery:
/* A range [start,end]. Lines are numbered starting at 0, and the
* ranges include start but exclude end. */
struct range {
long start, end;
};Perhaps these should be replaced with an unsigned value instead, just to be clear that negative values do not make sense in this context?
Or rather, do we want to enforce start < end if they are to be valuable? We apparently do assert() this in range_set_check_invariants() for all that does.
The current behavior of showing only the first line is a strange byproduct of allowing these odd ranges, but we may want to keep it for consistency. That would imply we allow start <= end, and auto-correct these negative values to have end = start. (This does not fix the 300,-1 case, but that is strange enough to be considered a bug, right?)
Thanks, -Stolee