Re: Re: [PATCH 1/3] checkout.c: add strict usage of -- before file_path
On 13/05/2018 00:03, Duy Nguyen wrote: On Sun, May 13, 2018 at 4:23 AM, Dannier Castro L <dannie...@gmail.com> wrote: For GIT new users, this complicated versatility of could be very confused, also considering that actually the flag '--' is completely useless (added or not, there is not any difference for this command), when the same program messages promote the use of this flag. I would like an option to revert back to current behavior. I'm not a new user. I know what I'm doing. Please don't make me type more. And '--" is not completely useless. If you have and with the same name, you have to give "--" to to tell git what the first argument means. Sure Duy, you're right, probably "completely useless" is not the correct definition, even according with the code I didn't find another useful case that is not file and branch with the same name. The program is able to know the type using only the name, turning "--" into an extra flag in most of cases. I think this solution could please you more: By default the configuration is the current, but the user has the chance to set this, for example: git config --global flag.strictdashdash true Thank you so much for the spent time reviewing the patch, this is my first one in this repository. -Dannier CL
[PATCH 3/3] doc: update doc for strict usage of -- checkout
The flag '--' must always be before any file path when command is used. The main documentation about the usage of command is updated to include the strict usage of the flag '--' so that the user can specify file names over branch names. Signed-off-by: Dannier Castro L <dannie...@gmail.com>: --- Documentation/git-checkout.txt | 20 ++-- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index ca5fc9c..8360a98 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -12,9 +12,9 @@ SYNOPSIS 'git checkout' [-q] [-f] [-m] --detach [] 'git checkout' [-q] [-f] [-m] [--detach] 'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] ] [] -'git checkout' [-f|--ours|--theirs|-m|--conflict=
[PATCH 1/3] checkout.c: add strict usage of -- before file_path
Currently, is a complex command able to handle both
branches and files without any distintion other than their names,
taking into account that depending on the type (branch or file),
the functionality is completely different, the easier example:
$ git checkout # Switch from current branch to .
$ git checkout # Restore from HEAD, discarding
# changes if it's necessary.
$ git checkout -- # The same as the last one, only with an
# useless '--'.
For GIT new users, this complicated versatility of could
be very confused, also considering that actually the flag '--' is
completely useless (added or not, there is not any difference for
this command), when the same program messages promote the use of
this flag.
Regarding the 's power to overwrite any file, discarding
changes if they exist without some way of recovering them, the
solution propuses that the usage of '--' is strict before to
specify the file(s) path(s) for any command (including
all types of flags), as a "defense barrier" to make sure about
user's knowledge and intension running .
The solution consists in detect '--' into command args, allowing
the discard of changes and considering the following names as
file paths, otherwise, they are branch names.
Signed-off-by: Dannier Castro L <dannie...@gmail.com>
---
builtin/checkout.c | 30 +-
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index d76e13c..ec577b3 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -40,6 +40,7 @@ struct checkout_opts {
int ignore_skipworktree;
int ignore_other_worktrees;
int show_progress;
+ int discard_changes;
const char *new_branch;
const char *new_branch_force;
@@ -885,8 +886,8 @@ static int parse_branchname_arg(int argc, const char **argv,
/*
* case 1: git checkout -- []
*
-*must be a valid tree, everything after the '--' must be
-* a path.
+*must be a valid tree. '--' must always be before any path,
+* it means, everything after the '--' must be a path.
*
* case 2: git checkout -- []
*
@@ -900,26 +901,28 @@ static int parse_branchname_arg(int argc, const char
**argv,
* omit at most one side), and if there is a unique merge base
* between A and B, A...B names that merge base.
*
-* (b) If is _not_ a commit, either "--" is present
-* or is not a path, no -t or -b was given, and
-* and there is a tracking branch whose name is
-* in one and only one remote, then this is a short-hand to
-* fork local from that remote-tracking branch.
+* (b) If is _not_ a commit, either "--" is present,
+* no -t or -b was given, and there is a tracking branch
+* whose name is in one and only one remote,
+* then this is a short-hand to fork local from
+* that remote-tracking branch.
*
* (c) Otherwise, if "--" is present, treat it like case (1).
*
* (d) Otherwise :
* - if it's a reference, treat it like case (1)
-* - else if it's a path, treat it like case (2)
* - else: fail.
*
-* case 4: git checkout
+*can never be a path (at least without '--' before).
+*
+* case 4: git checkout --
*
* The first argument must not be ambiguous.
* - If it's *only* a reference, treat it like case (1).
-* - If it's only a path, treat it like case (2).
* - else: fail.
*
+*can never be a path (at least without '--' before).
+*
*/
if (!argc)
return 0;
@@ -928,6 +931,7 @@ static int parse_branchname_arg(int argc, const char **argv,
dash_dash_pos = -1;
for (i = 0; i < argc; i++) {
if (!strcmp(argv[i], "--")) {
+ opts->discard_changes = 1;
dash_dash_pos = i;
break;
}
@@ -957,8 +961,8 @@ static int parse_branchname_arg(int argc, const char **argv,
(check_filename(opts->prefix, arg) || !no_wildcard(arg)))
recover_with_dwim = 0;
/*
-* Accept "git checkout foo" and "git checkout foo --"
-* as candidates for dwim.
+* Accept "git checkout foo_branch" and
+* "git checkout foo_branch --" as candidates for dwim.
*/
if (!(argc == 1 && !has_dash_dash) &&
!(argc == 2 && has_das
[PATCH 2/3] test: update tests for strict usage of -- checkout
The flag '--' must always be before any file path when command is used. The list of 34 test files updated is shown: t0021-conversion.sh t0027-auto-crlf.sh t1011-read-tree-sparse-checkout.sh t1050-large.sh t1051-large-conversion.sh t2009-checkout-statinfo.sh t2010-checkout-ambiguous.sh t2013-checkout-submodule.sh t2016-checkout-patch.sh t2022-checkout-paths.sh t2028-worktree-move.sh t2030-unresolve-info.sh t3001-ls-files-others-exclude.sh t3509-cherry-pick-merge-df.sh t3510-cherry-pick-sequence.sh t4015-diff-whitespace.sh t4124-apply-ws-rule.sh t5403-post-checkout-hook.sh t6007-rev-list-cherry-pick-file.sh t6026-merge-attr.sh t6030-bisect-porcelain.sh t6111-rev-list-treesame.sh t7001-mv.sh t7008-grep-binary.sh t7201-co.sh t7300-clean.sh t7501-commit.sh t7502-commit.sh t7607-merge-overwrite.sh t7810-grep.sh t7811-grep-open.sh t8006-blame-textconv.sh t9010-svn-fe.sh t8003-blame-corner-cases.sh Signed-off-by: Dannier Castro L <dannie...@gmail.com> --- t/t0021-conversion.sh| 12 ++-- t/t0027-auto-crlf.sh | 4 ++-- t/t1011-read-tree-sparse-checkout.sh | 4 ++-- t/t1050-large.sh | 2 +- t/t1051-large-conversion.sh | 4 ++-- t/t2009-checkout-statinfo.sh | 6 +++--- t/t2010-checkout-ambiguous.sh| 4 ++-- t/t2013-checkout-submodule.sh| 4 ++-- t/t2016-checkout-patch.sh| 9 + t/t2022-checkout-paths.sh| 4 ++-- t/t2028-worktree-move.sh | 2 +- t/t2030-unresolve-info.sh| 6 +++--- t/t3001-ls-files-others-exclude.sh | 2 +- t/t3420-rebase-autostash.sh | 2 +- t/t3510-cherry-pick-sequence.sh | 4 ++-- t/t3910-mac-os-precompose.sh | 4 ++-- t/t4015-diff-whitespace.sh | 4 ++-- t/t4117-apply-reject.sh | 2 +- t/t4124-apply-ws-rule.sh | 16 t/t5403-post-checkout-hook.sh| 2 +- t/t6007-rev-list-cherry-pick-file.sh | 2 +- t/t6026-merge-attr.sh| 2 +- t/t6030-bisect-porcelain.sh | 2 +- t/t6038-merge-text-auto.sh | 2 +- t/t6050-replace.sh | 2 +- t/t6111-rev-list-treesame.sh | 2 +- t/t7001-mv.sh| 2 +- t/t7008-grep-binary.sh | 2 +- t/t7201-co.sh| 8 t/t7300-clean.sh | 2 +- t/t7501-commit.sh| 10 +- t/t7502-commit.sh| 2 +- t/t7607-merge-overwrite.sh | 4 ++-- t/t7810-grep.sh | 2 +- t/t7811-grep-open.sh | 2 +- t/t8003-blame-corner-cases.sh| 2 +- t/t8006-blame-textconv.sh| 2 +- t/t9010-svn-fe.sh| 2 +- 38 files changed, 71 insertions(+), 78 deletions(-) diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh index 46f8e58..9f4955a 100755 --- a/t/t0021-conversion.sh +++ b/t/t0021-conversion.sh @@ -380,7 +380,7 @@ test_expect_success PERL 'required process filter should filter data' ' git commit -m "test commit 2" && rm -f test2.r "testsubdir/test3 '\''sq'\'',\$x=.r" && - filter_git checkout --quiet --no-progress . && + filter_git checkout --quiet --no-progress -- . && cat >expected.log <<-EOF && START init handshake complete @@ -466,7 +466,7 @@ test_expect_success PERL 'required process filter should be used only for "clean rm test.r && - filter_git checkout --quiet --no-progress . && + filter_git checkout --quiet --no-progress -- . && # If the filter would be used for "smudge", too, we would see # "IN: smudge test.r 57 [OK] -- OUT: 57 . [OK]" here cat >expected.log <<-EOF && @@ -580,7 +580,7 @@ test_expect_success PERL 'process filter should restart after unexpected write f rm -f *.r && rm -f debug.log && - git checkout --quiet --no-progress . 2>git-stderr.log && + git checkout --quiet --no-progress -- . 2>git-stderr.log && grep "smudge write error at" git-stderr.log && grep "error: external filter" git-stderr.log && @@ -630,7 +630,7 @@ test_expect_success PERL 'process filter should not be restarted if it signals a git add . && rm -f *.r && - filter_git checkout --quiet --no-progress . && + filter_git checkout --quiet --no-progress -- . && cat >expected.log
