verify_filename(): ask the caller to chose the kind of diagnosis

verify_filename() can be called in two different contexts. Either we
just tried to interpret a string as an object name, and it fails, so
we try looking for a working tree file (i.e. we finished looking at
revs that come earlier on the command line, and the next argument
must be a pathname), or we _know_ that we are looking for a
pathname, and shouldn't even try interpreting the string as an
object name.

For example, with this change, we get:

$ git log COPYING HEAD:inexistant
fatal: HEAD:inexistant: no such path in the working tree.
Use '-- <path>...' to specify paths that do not exist locally.
$ git log HEAD:inexistant
fatal: Path 'inexistant' does not exist in 'HEAD'

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/grep.c b/builtin/grep.c
index 9ce064ac1131e9a93383f568bb6f567791740b77..d56555651438e693dd9d13d4ab6de863d4daae29 100644 (file)
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1045,7 +1045,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
        if (!seen_dashdash) {
                int j;
                for (j = i; j < argc; j++)
        if (!seen_dashdash) {
                int j;
                for (j = i; j < argc; j++)

-                       verify_filename(prefix, argv[j]);
+                       verify_filename(prefix, argv[j], j == i);

        }
 
        paths = get_pathspec(prefix, argv + i);
        }
 
        paths = get_pathspec(prefix, argv + i);

diff --git a/builtin/reset.c b/builtin/reset.c
index 8c2c1d52a227334a3d6456bf0989cd561628ffa0..4cc34c908446fe2d3db5acf315e47f2768ba07bd 100644 (file)
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -285,7 +285,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
                        rev = argv[i++];
                } else {
                        /* Otherwise we treat this as a filename */
                        rev = argv[i++];
                } else {
                        /* Otherwise we treat this as a filename */

-                       verify_filename(prefix, argv[i]);
+                       verify_filename(prefix, argv[i], 1);

                }
        }
 
                }
        }
 

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 98d1cbeccacc4b22dee88dab3d0154a3f70afe81..3e2f5bd44afb1adee53e554153285f380848c5d9 100644 (file)
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -486,7 +486,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 
                if (as_is) {
                        if (show_file(arg) && as_is < 2)
 
                if (as_is) {
                        if (show_file(arg) && as_is < 2)

-                               verify_filename(prefix, arg);
+                               verify_filename(prefix, arg, 0);

                        continue;
                }
                if (!strcmp(arg,"-n")) {
                        continue;
                }
                if (!strcmp(arg,"-n")) {


@@ -732,7 +732,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
                as_is = 1;
                if (!show_file(arg))
                        continue;
                as_is = 1;
                if (!show_file(arg))
                        continue;

-               verify_filename(prefix, arg);
+               verify_filename(prefix, arg, 1);

        }
        if (verify) {
                if (revs_count == 1) {
        }
        if (verify) {
                if (revs_count == 1) {

diff --git a/cache.h b/cache.h
index 10afd71d435920a3cc2152208a06058791ea7a04..35383e3f66977b80736e8418fbb512f3a8d3e8ee 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -452,7 +452,9 @@ extern const char *setup_git_directory(void);
 extern char *prefix_path(const char *prefix, int len, const char *path);
 extern const char *prefix_filename(const char *prefix, int len, const char *path);
 extern int check_filename(const char *prefix, const char *name);
 extern char *prefix_path(const char *prefix, int len, const char *path);
 extern const char *prefix_filename(const char *prefix, int len, const char *path);
 extern int check_filename(const char *prefix, const char *name);

-extern void verify_filename(const char *prefix, const char *name);
+extern void verify_filename(const char *prefix,
+                           const char *name,
+                           int diagnose_misspelt_rev);

 extern void verify_non_filename(const char *prefix, const char *name);
 
 #define INIT_DB_QUIET 0x0001
 extern void verify_non_filename(const char *prefix, const char *name);
 
 #define INIT_DB_QUIET 0x0001

diff --git a/revision.c b/revision.c
index 064e35108478431e82ec08464203fe119f24c1d6..b3c61e1423deef88791309506a10d85ac55bfa9a 100644 (file)
--- a/revision.c
+++ b/revision.c
@@ -1755,7 +1755,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
                         * but the latter we have checked in the main loop.
                         */
                        for (j = i; j < argc; j++)
                         * but the latter we have checked in the main loop.
                         */
                        for (j = i; j < argc; j++)

-                               verify_filename(revs->prefix, argv[j]);
+                               verify_filename(revs->prefix, argv[j], j == i);

 
                        append_prune_data(&prune_data, argv + i);
                        break;
 
                        append_prune_data(&prune_data, argv + i);
                        break;

diff --git a/setup.c b/setup.c
index 61c22e6becc1e49f1e92c916a4b8badd30a9cb2f..d121b6ab77cb40afff69cb9003be62033a12802f 100644 (file)
--- a/setup.c
+++ b/setup.c
@@ -53,11 +53,17 @@ int check_filename(const char *prefix, const char *arg)
        die_errno("failed to stat '%s'", arg);
 }
 
        die_errno("failed to stat '%s'", arg);
 }
 

-static void NORETURN die_verify_filename(const char *prefix, const char *arg)
+static void NORETURN die_verify_filename(const char *prefix,
+                                        const char *arg,
+                                        int diagnose_misspelt_rev)

 {
        unsigned char sha1[20];
        unsigned mode;
 
 {
        unsigned char sha1[20];
        unsigned mode;
 

+       if (!diagnose_misspelt_rev)
+               die("%s: no such path in the working tree.\n"
+                   "Use '-- <path>...' to specify paths that do not exist locally.",
+                   arg);

        /*
         * Saying "'(icase)foo' does not exist in the index" when the
         * user gave us ":(icase)foo" is just stupid.  A magic pathspec
        /*
         * Saying "'(icase)foo' does not exist in the index" when the
         * user gave us ":(icase)foo" is just stupid.  A magic pathspec


@@ -80,14 +86,29 @@ static void NORETURN die_verify_filename(const char *prefix, const char *arg)
  * as true, because even if such a filename were to exist, we want
  * it to be preceded by the "--" marker (or we want the user to
  * use a format like "./-filename")
  * as true, because even if such a filename were to exist, we want
  * it to be preceded by the "--" marker (or we want the user to
  * use a format like "./-filename")

+ *
+ * The "diagnose_misspelt_rev" is used to provide a user-friendly
+ * diagnosis when dying upon finding that "name" is not a pathname.
+ * If set to 1, the diagnosis will try to diagnose "name" as an
+ * invalid object name (e.g. HEAD:foo). If set to 0, the diagnosis
+ * will only complain about an inexisting file.
+ *
+ * This function is typically called to check that a "file or rev"
+ * argument is unambiguous. In this case, the caller will want
+ * diagnose_misspelt_rev == 1 when verifying the first non-rev
+ * argument (which could have been a revision), and
+ * diagnose_misspelt_rev == 0 for the next ones (because we already
+ * saw a filename, there's not ambiguity anymore).

  */
  */

-void verify_filename(const char *prefix, const char *arg)
+void verify_filename(const char *prefix,
+                    const char *arg,
+                    int diagnose_misspelt_rev)

 {
        if (*arg == '-')
                die("bad flag '%s' used after filename", arg);
        if (check_filename(prefix, arg))
                return;
 {
        if (*arg == '-')
                die("bad flag '%s' used after filename", arg);
        if (check_filename(prefix, arg))
                return;

-       die_verify_filename(prefix, arg);
+       die_verify_filename(prefix, arg, diagnose_misspelt_rev);

 }
 
 /*
 }
 
 /*

diff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh
index e81dcd6de6639227e5e74a978aa193286e449ec8..c5cb77a0e1f34ac46dd8727341948389624e8f38 100755 (executable)
--- a/t/t1506-rev-parse-diagnosis.sh
+++ b/t/t1506-rev-parse-diagnosis.sh
@@ -174,7 +174,7 @@ test_expect_success 'relative path when startup_info is NULL' '
 test_expect_success '<commit>:file correctly diagnosed after a pathname' '
        test_must_fail git rev-parse file.txt HEAD:file.txt 1>actual 2>error &&
        test_i18ngrep ! "exists on disk" error &&
 test_expect_success '<commit>:file correctly diagnosed after a pathname' '
        test_must_fail git rev-parse file.txt HEAD:file.txt 1>actual 2>error &&
        test_i18ngrep ! "exists on disk" error &&

-       test_i18ngrep "unknown revision or path not in the working tree" error &&
+       test_i18ngrep "no such path in the working tree" error &&

        cat >expect <<-\EOF &&
        file.txt
        HEAD:file.txt
        cat >expect <<-\EOF &&
        file.txt
        HEAD:file.txt