Hi Junio
On 22/01/2021 21:59, Junio C Hamano wrote:
Phillip Wood <phillip.wood123@xxxxxxxxx> writes:
+static int is_range(const char *range)
+{
+ return !!strstr(range, "..");
+}
If the user wrongly passes two arguments referring to single commits
with `:/<text>` or `@{/<text>}` where text contains ".." this will
give a false positive.
True. I do not think this aims to be complete revision parser in
the first place, though.
Yes but it affects the error message given to the user. If I run
git range-diff $(git rev-parse HEAD^{/..q}) $(git rev-parse HEAD^{/..x})
It fails immediately with
fatal: no .. in range: 'b60863619cf47b2e1e891c2376bd4f6da8111ab1'
This patch improves the error message to say it is not a range
but
git range-diff HEAD^{/..q} HEAD^{/..x}
runs for several minutes without producing any output and eventually
fails with
fatal: Out of memory, malloc failed (tried to allocate 33846432676 bytes)
So I think it would be helpful to be more careful here.
Best Wishes
Phillip
It is tempting to at least idly speculate if an approach to run
setup_revisions() on argument is_range() takes and checking the
result would yield a more practical solution. I would imagine that
we would want to see in the resulting revs.pending has at least one
positive and one negative, and none of them have SYMMETRIC_LEFT set
in their .flags word.
Side note: Strictly speaking, people could wish "rev" to mean
"everything reachable from the rev, down to root", so
requiring one negative may technically be a wrong
thing, but in the context of "range-diff", I am not
sure how useful such a behaviour would be.
