Re: [PATCH/RFC] revision: Show friendlier message.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Leila <muhtasib@xxxxxxxxx> writes:

>>  2. Make setup_revisions() expose got_rev_arg to its callers
>>    (e.g. move it to struct rev_info);
>
> Do you mean have got_rev_args be a wrapper of argc and argv?

No.  The setup_revisions() function knows if it saw a revision
argument from the command line, but currently uses got_rev_args
local variable, so the caller would not be able to tell.  I was
suggesting to use "struct rev_info *revs" that goes in and comes out
of the function to convey that information back to the caller.

But it turns out that it is not even needed.  Read on.

> Or is it just a mechanism to set a signal that the calling command is
> 'log', so that I can do something about it without checking argv[0]?

Didn't I already say not to switch on argv[0] in deeper side of the
callchain?

>>  3. If you did not pass HEAD in opt->def and setup_revisions() said
>>    it did not "got_rev_arg", give whatever error message that you
>>    think is more user friendly.
>>
>
> Sure, I can do this. Note: just to confirm the message/exit will still
> come from inside of setup_revisions()?

No.  I do not want any patch that butchers setup_revisions() with
any of this kind of UI issues.

Something like this, I think, would work.  After all, we already
have a way to expose the revs we got from the command line to the
caller.

The "bad HEAD and no revs..." part, if we choose not to even error
on this, can be removed.

Also other cmd_frotz() functions in the same file might want to use
the s/"HEAD"/default_to_head_if_exists()/ conversion.

 builtin/log.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/builtin/log.c b/builtin/log.c
index 4f1b42a..6ecf344 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -355,6 +355,15 @@ static int git_log_config(const char *var, const char *value, void *cb)
 	return git_diff_ui_config(var, value, cb);
 }
 
+static const char *default_to_head_if_exists(void)
+{
+	unsigned char sha1[20];
+	if (resolve_ref_unsafe("HEAD", sha1, 1, NULL))
+		return "HEAD";
+	else
+		return NULL;
+}
+
 int cmd_whatchanged(int argc, const char **argv, const char *prefix)
 {
 	struct rev_info rev;
@@ -553,8 +562,15 @@ int cmd_log(int argc, const char **argv, const char *prefix)
 	init_revisions(&rev, prefix);
 	rev.always_show_header = 1;
 	memset(&opt, 0, sizeof(opt));
-	opt.def = "HEAD";
+	opt.def = default_to_head_if_exists();
 	cmd_log_init(argc, argv, prefix, &rev, &opt);
+
+	if (!opt.def && !rev.cmdline.nr) {
+		/*
+		 * bad HEAD and no revs on the command line
+		 */
+		warning("Nothing to show...");
+	}
 	return cmd_log_walk(&rev);
 }
 
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]