[PATCH] diff-cache path restriction fix.

[PATCH] diff-cache path restriction fix.

[Junio C Hamano]

It advertises the path restriction in documentation and usage
string, but the argument parsing code was not updated and was
causing it to refuse to run.  One liner fix is here.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
# - linus: git-update-cache: allow dot-files
# + (working tree)
diff --git a/diff-cache.c b/diff-cache.c
--- a/diff-cache.c
+++ b/diff-cache.c
@@ -164,7 +164,7 @@ int main(int argc, const char **argv)
 	int ret;
 
 	read_cache();
-	while (argc > 2) {
+	while (1 < argc && argv[1][0] == '-') {
 		const char *arg = argv[1];
 		argv++;
 		argc--;

Re: [PATCH] diff-cache path restriction fix.

[Linus Torvalds]

On Tue, 24 May 2005, Junio C Hamano wrote:
>
> It advertises the path restriction in documentation and usage
> string, but the argument parsing code was not updated and was
> causing it to refuse to run.  One liner fix is here.

No, it's more broken than that. Look at how it uses "argv[1]" for the tree
SHA1, then does "argv++" and then uses "argv[1]" (which is a totally
different argument entirely) for error reporting when the tree SHA1 is bad

> -	while (argc > 2) {
> +	while (1 < argc && argv[1][0] == '-') {

Btw, that "1 < argc" order is very unintuitive to most humans. Like it or 
not, people get used to things one way, and have a hard time seeing what 
it means when it's the other way around.

And when people have a hard time seeing what it means, you get more bugs.

This is why it is _not_ better to do 

	if (1 == a)

like some people teach, even if that protects against the "single equal
sign" bug. There are better ways to protect against that one bug (like
having compiler warnings enabled) that don't make the code less obvious.

		Linus

Re: [PATCH] diff-cache path restriction fix.

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> No, it's more broken than that.

I'll take a look at this later and submit an update, but an OT
point I feel I should address.

LT> Btw, that "1 < argc" order is very unintuitive to most humans.

Yeah?  Not to people around where I come from, I do not know
why.  It is not done for the assignment confusion avoidance
"1==a".

The comparison lists things in the ascending order from left to
right.  The fact that 1 comes before argc on that line of code
visually makes it obvious that I am talking about argc being
larger than one and that is the reason.  I'd write (argc < 4)
not (4 > argc) for the same reason.

Re: [PATCH] diff-cache path restriction fix.

[Linus Torvalds]

On Tue, 24 May 2005, Junio C Hamano wrote:

> >>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:
> 
> LT> No, it's more broken than that.
> 
> I'll take a look at this later and submit an update, but an OT
> point I feel I should address.

I checked in the fixed arg parsing already ;)

> The comparison lists things in the ascending order from left to
> right.  The fact that 1 comes before argc on that line of code
> visually makes it obvious that I am talking about argc being
> larger than one and that is the reason.  I'd write (argc < 4)
> not (4 > argc) for the same reason.

Hmm. According to that logic, ">" and ">=" is superfluous.

Also, what language do you actually speak? Every human language I know
(admittedly, apart from Finnish they are all related) tends to say things
like "if you have more than four children, you're in trouble", rather than
saying "if four is less than the number of children you have".

Notice? People compare something _to_ a number. They don't compare a
number to something. We say "if more than four", we don't say "if four is
less".

Sadly, I can't get google to match "1 < x" and "x > 1" (it just considers
all special characters to be basically whitespace, so "1 < x" matches "1
x" and "1.x" according to google. Usually google is a good way to get a
feel for how common some phrase is, but not on things like this.

		Linus

Re: [PATCH] diff-cache path restriction fix.

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> I checked in the fixed arg parsing already ;)

Thanks.

LT> Hmm. According to that logic, ">" and ">=" is superfluous.

Yes, I was trained by Paul Eggert (me says that proudly).

Practically speaking, the only time I deliberately used > and >=
was when I was doing some dialect of SQL that always wanted
literal on fixed side and column on the other; I do not remember
which was which and whose SQL anymore.

Of course I sometimes end up using them when I am trying to
match the style of existing code.  However, for that particular
comparison in diff-cache, there weren't any other around there
to match, other than the "if (argc < 2 || ...)" after the loop,
which was what I myself wrote so it does not count.

LT> Also, what language do you actually speak?

Japanese.

I have not thought about that kind of relationship between the
natural language and if() expression at all, and I am certainly
not claiming comparing it the logic way is natural in Japanese.
I think it probably isn't.

LT> ... Usually google is a good way to get a feel for how
LT> common some phrase is, but not on things like this.

If you feel strongly about this, just write it in coding-style
document and I'll follow whatever you tell me while I am coding
for this project.  Honestly, I do not particularly care how
common that is in the wider world outside.

Re: [PATCH] diff-cache path restriction fix.

[Russ Allbery]

Junio C Hamano <junkio@cox.net> writes:

> Yes, I was trained by Paul Eggert (me says that proudly).

> Practically speaking, the only time I deliberately used > and >=
> was when I was doing some dialect of SQL that always wanted
> literal on fixed side and column on the other; I do not remember
> which was which and whose SQL anymore.

> Of course I sometimes end up using them when I am trying to
> match the style of existing code.  However, for that particular
> comparison in diff-cache, there weren't any other around there
> to match, other than the "if (argc < 2 || ...)" after the loop,
> which was what I myself wrote so it does not count.

My prior programming experience has taught me to read argv > 1 as an
assertion about argv, as opposed to 1 < argv, which would be an assertion
about 1.  In other words, as I code, I'm generally thinking about testing
a variable against some sort of boundary condition (which may or may not
be itself variable), and the thing that I'm testing goes first, followed
by the test.  As a result, 1 < argv throws me for a moment, since on first
read it seems to imply the programmer was expecting the value of 1 to
change.

-- 
Russ Allbery (rra@stanford.edu)             <http://www.eyrie.org/~eagle/>

Re: [PATCH] diff-cache path restriction fix.

[Junio C Hamano]

I do not think there is any right or wrong in this discussion,
so I would not make any more comment on this topic for now.

Your interpretation that "(a cmp-op b) is an assertion on a" is
one valid interpretation of a boolean expression.  I would
understand it if you say you are used to think of it as an
assertion about the left hand side.  I just do not think of it
that way.  Rather, to me, "(a cmp-op b)" (or an boolean
expression in any shape for that matter) as a whole is an
assertion, and it is simply easier for me if a and b are ordered
from left to right, to visually match ascending order.

But that is only because I am used to read programs written that
way.  Just like you are used to think of these expressions about
assertions of the left hand side.

Re: [PATCH] diff-cache path restriction fix.

[Linus Torvalds]

On Tue, 24 May 2005, Junio C Hamano wrote:
>
> LT> Also, what language do you actually speak?
> 
> Japanese.

It is possible it is cultural. I certainly find it harder to read the 
"unexpected" way. 

But maybe it's just me. I also have a _really_ hard time with reading
"unless(x)" (aka "if (!(x))"), that perl programmers seem to use. 

		Linus

Re: [PATCH] diff-cache path restriction fix.

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> On Tue, 24 May 2005, Junio C Hamano wrote:
>> 
LT> Also, what language do you actually speak?
>> 
>> Japanese.

LT> It is possible it is cultural. I certainly find it harder to read the 
LT> "unexpected" way. 

I doubt it is Japanese vs Western kind of cultural.  When I said
"people around me", that set of people did not include a single
Japanese.  I meant people who worked with the person I was
trained by to use this style.  Cultural, maybe, but that is
programming culture and definitely not natural language culture
in my case.

Here is a quick and dirty experiment which showed quite an
interesting statistics.  It counts number of '<' and '>' in the
program text, after stripping out ptr->deref (yes it catches
#include <stdlib.h>, but they even out and it also catches
whatever is in comments, but this is just a Q&D stats for fun).
The percentage is how close the program text is to "visually
ordered" style:

    $ cat count-compare.perl
    #!/usr/bin/perl -w 

    for my $filename (@ARGV) {
        open I, '<', $filename;
        my ($lt, $gt) = (0, 0);
        while (<I>) {
            s/->//g; # pointer deref not comparison
            $lt += tr/</</; # count
            $gt += tr/>/>/; # count
        }
        close I;
        my $ratio = ($lt / ($lt + $gt)) * 100;
        printf "'<' (%d) '>' (%d) (%d%%) %s\n", $lt, $gt, $ratio, $filename;
    }
    $ perl count-compare.perl diff*.c
    '<' (9) '>' (4) (69%) diff-cache.c
    '<' (20) '>' (26) (43%) diff-delta.c
    '<' (8) '>' (1) (88%) diff-files.c
    '<' (12) '>' (1) (92%) diff-helper.c
    '<' (11) '>' (11) (50%) diff-tree.c
    '<' (28) '>' (10) (73%) diff.c
    '<' (3) '>' (0) (100%) diffcore-pathspec.c
    '<' (2) '>' (0) (100%) diffcore-pickaxe.c
    '<' (22) '>' (6) (78%) diffcore-rename.c

This clearly shows that diff-delta.c does not have my code at
all.  Most of the others have been touched moderately to heavily
by me, or in some cases done entirely by me.

Personally, what I find most interesting is that diff-tree.c is
something you did quite a lot of nice features (and hence
something I was afraid to touch), and the number clearly shows
my hesitation.  It does not have as many '<' as it would have
had, if I had mucked with it as freely as I did to the others.

Re: [PATCH] diff-cache path restriction fix.

[Ingo Molnar]

* Linus Torvalds <torvalds@osdl.org> wrote:

> On Tue, 24 May 2005, Junio C Hamano wrote:
> >
> > LT> Also, what language do you actually speak?
> > 
> > Japanese.
> 
> It is possible it is cultural. I certainly find it harder to read the 
> "unexpected" way.

i'm quite sure it's related to ambiguity. The main problem for the human 
brain when reading code is ambiguity of expression - ambiguity triggers 
'logic' areas of the brain, instead of the 'visual automation' portions
of the brain.  Like it or not, most of the code reading we do is all
automatism, if we had to _think_ about the visual structure of the code
we'd be much less effective.

The moment we fall out of automation (you go to the bathroom in the 
morning but the toothpaste is empty, or you are in the shop and the 
coffee area got moved to another place), we feel unease. Thinking during 
normally routine activities means problems, it means distraction, it 
meant larger reaction times in the jungle for millions of years, and 
that's why the built-in unease.  Thinking generates unease _especially_ 
if you dont expect it and dont want it - even if you happen to be Albert 
Einstein or Linus Torvalds ;) [And it's way too easy to let the 
autopilot drive all the time - there are people who stop thinking in 
their childhood and autopilot through life.]

Coding styles are mostly there to reduce the syntactic ambiguities of 
computer languages (and hence reducing parsing complexity), and thus to 
make it easier for the human brain to parse them - and thus to give more 
brain capacity for the real thinking.

the other interesting question is, why do most coders pick the 'x < 1' 
variant? I'm quite sure that's due to most of us having learned coding 
via operators. It's "x := 1" and "x /= 2", where the mirror image is not 
valid - and we extend that expectation to ambiguous operators too. It's 
the more complex entitity (the variable) that we think about first, and 
then comes the less complex entity. But if someone has a strong math 
background (Junio?) then the "1 < x < 5" syntax could be the natural 
thing he got used to.

so as long as the actual expressions are used in an unambiguous way, 
it's fine and it's part of the coding style and it's all a matter of 
getting used to it. Junio's method is just as unambiguous. How quickly
you can adopt to another coding style is directly related to how
practiced you are at it, but it also depends on your fundamental
abstraction abilities. The overwhelming majority of coders dont "switch"
a coding style that easily, but e.g. people who maintain a large number
of packages or use lots of languages are very good at it. You have the
luxory to be able to read your own coding style all day so it's pretty
natural to feel unease if something else comes along :)

	Ingo

Re: [PATCH] diff-cache path restriction fix.

[Linus Torvalds]

On Wed, 25 May 2005, Ingo Molnar wrote:
>
> Like it or not, most of the code reading we do is all automatism, if we
> had to _think_ about the visual structure of the code we'd be much less
> effective.

Absolutely. And you _should_ like it, because it means that you can 
concentrate on the big problems.

I suspect I react more strongly to some coding style issues than most, 
exactly because I've been so involved with just one language and one style 
for so long that I've got more of an auto-pilot than most: normally my 
brain just coasts past 99% of all code, without having to be very 
conscious about it at all, exactly because the patterns have become so 
ingrained.

In contrast, somebody who works on different projects and with many
different languages is likely to have many more patterns, but they are
probably less deep, so breaking them isn't as disturbing.

>             But if someone has a strong math background (Junio?) then
> the "1 < x < 5" syntax could be the natural thing he got used to.

Actually, even in math (in fact, very _much_ in math) I'd say that people
always aim to put the variable on the left side, and you very much tend to
try to aim for canonical formats (sort by exponent and variable name). 
You'd never see anybody write "y + x = 5" or "x + x^2", because there's a 
canonical format for it, and people use it.

The "1 < x < 5" syntax is very much the odd man out, and that order for
the first part is _only_ used for two-sided comparisons afaik. Math people
(at least all math texts I've ever seen, and I had math as a strong minor
although admittedly that is some time ago) will invariably write the
variable on the left side if there's just one comparison.

In fact, in math, I'd expect a person to often tend to take the canonical
format even further, and sort _everything_ to the left side, and aim to
leave the right side totally empty (ie "0"), exactly because that makes
certain combinations much easier. So it wouldn't surprise me at all to see
a math person change "x > 1" into "x - 1 > 0", but it _would_ surprise me
to see a math person change it into "1 < x" unless it is to combine it
with _another_ comparison with "x".

				Linus

Re: [PATCH] diff-cache path restriction fix.

[Junio C Hamano]

>>>>> "IM" == Ingo Molnar <mingo@elte.hu> writes:

IM> ... But if someone has a strong math 
IM> background (Junio?) then the "1 < x < 5" syntax could be the natural 
IM> thing he got used to.

I was trained by somebody with a strong math background, but I
suspect he insisted that ordering not from math reasons.  The
reason given to me was "visual ordering".

I suck at math myself.  I even get confused when people do

    return !!some_function();

Re: [PATCH] diff-cache path restriction fix.

[Thomas Glanzmann]

Hello,

>     return !!some_function();

I actually wanted to send a diff to get rid of this thing from
sha_file.c :-)

	Thomas

Re: [PATCH] diff-cache path restriction fix.

[Matthias Urlichs]

Hi, Ingo Molnar wrote:

> But if someone has a strong math background
> (Junio?) then the "1 < x < 5" syntax could be the natural thing he got
> used to.

Or Python -- and I assume there are more languages where this idiom works
correctly.

-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de

[OT] if (4 < number_of_children) you're in trouble

[Junio C Hamano]

I received this from my mentor tonight.  Although I said I am
not going to discuss this issue anymore, with his permission, I
would like to share where the "visual order" came from with the
list.

From: Paul Eggert <eggert@CS.UCLA.EDU>
Subject: Re: FYI: comparison order in boolean expressions
To: Junio C Hamano <junkio@cox.net>
Date: Sat, 28 May 2005 00:28:31 -0700

> http://marc.theaimsgroup.com/?l=git&m=111698274620068&w=2

Thanks!  That made my day.

You can forward the following to that mailing list if you like:


I learned the "textual order should reflect actual order" convention
from D. Val Schorre, one of the best programmers I've ever worked
with.  Val was credited by Donald E. Knuth as the first person to
seriously advocate goto-less programming.  In 1974 Knuth wrote:

  When I met Schorre in 1963, he told me of his radical ideas, and I
  didn't believe they would work....  In 1964 I challenged him to
  write a program for the eight-queens problem without using go to
  statements, and he responded with a program using recursive
  procedures and Boolean variables, very much like the program later
  published independently by Wirth.

  I was still not convinced....

Knuth's reaction to Schorre's ideas on avoiding gotos sounds a bit
like Linus's reaction to Schorre's ideas on comparison order, no?

Anyway, here's the reference, if you'd like to find out how Knuth's
story ends:

  Donald E. Knuth
  Structured Programming with go to Statements
  ACM Computing Surveys 6, 4 (December 1974), 261-301.
  http://portal.acm.org/citation.cfm?id=356640


PS.  The idea that textual form should reflect the underlying reality
goes all the way back to Gottfried Leibniz's alphabet of human thought
<http://en.wikipedia.org/wiki/Alphabet_of_human_thought>.  Leibniz was
a brilliant designer of notations -- one of the best the world has
ever seen -- and it is amusing to speculate what he would have done
with programming had he been born some time in the past few decades.
(Personally I suspect that Leibniz would have liked "1 < x".)

Re: [PATCH] diff-cache path restriction fix.

[Ingo Molnar]

* Linus Torvalds <torvalds@osdl.org> wrote:

> Hmm. According to that logic, ">" and ">=" is superfluous.
> 
> Also, what language do you actually speak? Every human language I know 
> (admittedly, apart from Finnish they are all related) tends to say 
> things like "if you have more than four children, you're in trouble", 
> rather than saying "if four is less than the number of children you 
> have".

[ add Hungarian to that short list - there it's a pretty natural thing 
to say "if four is less than the number of ..." in everyday life :-| 
Interestingly, having an insanely complex wacko weird language helps 
kids learn abstraction early, and results in an unusually high per 
capita proportion of scientists. If China adopted Finnish as a second 
language the global domination of the US would be history in 50 years
;-) ]

	Ingo

Re: [PATCH] diff-cache path restriction fix.

[Florian Weimer]

* Junio C. Hamano:

> LT> Btw, that "1 < argc" order is very unintuitive to most humans.
>
> Yeah?  Not to people around where I come from, I do not know
> why.  It is not done for the assignment confusion avoidance
> "1==a".

In a comparison, it's common to mention the most-varying part first.
If you do this consistently, it increases readability.