Samuel Bronson <naesten@xxxxxxxxx> writes:
>> If somebody has diff.orderfile configuration that points at a custom
>> ordering, and wants to send out a patch (or show a diff) with the
>> standard order, how would the "overriding" command line look like?
>> Would it be "git diff -O/dev/null"?
>
> It looks like that works ... and so do files that don't exist. What
> do you think should happen with -O file-that-does-not-exist, and how
> do you suppose it should be tested?
I think the original code is too loose on diagnosing errors.
Perhaps something like this is needed.
diff --git a/diffcore-order.c b/diffcore-order.c
index 23e9385..dd103e3 100644
--- a/diffcore-order.c
+++ b/diffcore-order.c
@@ -20,8 +20,11 @@ static void prepare_order(const char *orderfile)
return;
fd = open(orderfile, O_RDONLY);
- if (fd < 0)
+ if (fd < 0) {
+ if (errno != ENOENT || errno != ENOTDIR)
+ die(_("orderfile '%s' does not exist."));
return;
+ }
if (fstat(fd, &st)) {
close(fd);
return;
> After having fixed this, will /dev/null still work everywhere, or will
> we want a new diff flag to unset the option? (I see that "git diff
> /dev/null some-file" works fine with msysgit, which doesn't seem to
> actually be linked with MSYS, but I don't know *why* it works, and I
> don't know what other non-POSIXoid ports exist.)
I *think* it should be OK to use "-O/dev/null" for that purpose, but
the primary thing I was hinting at with the rhetoric question was
that it probably needs to be documented there.
> Also, I'm starting to wonder if I shouldn't split this into two patches:
>
> 1. diff: Add tests for -O flag
> 2. diff: Add diff.orderfile configuration variable
>
> (If so, I would obviously want to rewrite the above test to avoid the
> configuration option.)
Surely, and thanks.
>> EOF
>> cat >order_file_2 <<-\EOF &&
>
> I'd kind of prefer to keep a blank line between one EOF and the next
> cat, if that's okay with you.
Alright. Making it easier to spot the grouping, it would make it
easier to read.
>> Quoting the EOF like the above will help the readers by signaling
>> them that they do not have to wonder if there is some substitution
>> going on in the here text.
>
> Perhaps, but probably only after they've scrutinized their shell
> manuals to figure out what the - and the \ are for. (I had to check
> two: dash(1) wasn't clear enough for me about the quoting ...)
Yes and no. The no primarily comes from that nobody will stay to be
novice forever.
--
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