Refactor getpatch/getdiff functions and remove unnecessary fallback

[lunny]

Extract from #32786

git diff a..b is equal to git diff a b which is different from git diff a...b. For the pull request, we should always use git diff a...b.

dismiss

[wxiaoguang]

To be honest, I do not think the parseCompareArgs behavior is right.

It sometimes returns {"a..b"}, sometimes returns {"a", "b"}, does it really make sense? Why not make callers just pass "a...b" or "a..b"? And what's the use case for parsing a b (space separator)?

[lunny]

To be honest, I do not think the parseCompareArgs behavior is right.

It sometimes returns {"a..b"}, sometimes returns {"a", "b"}, does it really make sense? Why not make callers just pass "a...b" or "a..b"? And what's the use case for parsing a b (space separator)?

We can strictly parseCompareArgs only accept a...b and a..b and refuse a b if you think that's a problem. Although git supports both a..b and a b. I think it's just a habit or legacy reason.

[wxiaoguang]

and refuse a b if you think that's a problem. Although git supports both a..b and a b. I think it's just a habit or legacy reason.

Yes, it is absolutely a problem. It is not a right habit or legacy reason. Git does NOT support git diff "a b". It only accepts git diff "a" "b" as separate arguments (aka "slice" in Golang), not in ONE string.