git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Stefan Beller <stefanbeller@googlemail.com>
Cc: Ramkumar Ramachandra <artagnon@gmail.com>,
	Git List <git@vger.kernel.org>
Subject: Re: Remove old forgotten command: whatchanged
Date: Wed, 07 Aug 2013 23:39:17 -0700	[thread overview]
Message-ID: <7vpptobsa2.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <5202C109.6040209@googlemail.com> (Stefan Beller's message of "Wed, 07 Aug 2013 23:50:01 +0200")

Stefan Beller <stefanbeller@googlemail.com> writes:

> Well if we make sure the whatchanged command can easily be reproduced
> with the log command, we could add the missing parameters to it, hence
> no change for the user. (git whatchanged == git log --raw --no-merges or
> git log --wc [to be done yet]).
>
> So I did not mean to introduce a change for users!

I certainly did *not* read that from between the lines of what you
wrote:

+ int cmd_whatchanged(int argc, const char **argv, const char *prefix)
+ {
+ 	return cmd_log(argc, argv, prefix)
+ }

In principle, I agree that it is a good idea to try to share enough
code, while keeping the flexiblity and clarity of the code for
maintainability.

In the extreme, you could rewrite these two functions like so:

	static int cmd_lw_helper(
		int argc, const char **argv,
        	const char *prefix,
                int whoami)	
        {
		struct rev_info rev;
		struct setup_revision_opt opt;
        
		init_grep_defaults();
                git_config(git_log_config, NULL);
                init_revisions(&rev, prefix);
		if (whoami == 'l') { /* log */
	                rev.always_show_header = always_show_header;
		} else { /* whatchanged */
	                rev.diff = diff;
	                rev.simplify_history = simplify_history;
		}
                memset(opt, 0, sizeof(opt));
                opt.def = "HEAD";
                opt.revarg_opt = REVARG_COMMITTISH;
                cmd_log_init(argc, argv, prefix, &rev, &opt);
		if (whoami == 'w') {
			if (!rev.diffopt.output_format)
				rev.diffopt.output_format = DIFF_FORMAT_RAW;
		}
                return cmd_log_walk(&rev);
	}

        int cmd_log(int argc, const char **argv, const char *prefix)
	{
        	return cmd_lw_helper(argc, argv, prefix, 'l');
	}

        int cmd_whatchanged(int argc, const char **argv, const char *prefix)
	{
        	return cmd_lw_helper(argc, argv, prefix, 'w');
	}

but at that point, the cost you have to pay when you need to update
one of them but not the other becomes higher.

As whatchanged is kept primarily for people who learned Git by word
of mouth reading the kernel mailing list and are used to that
command.  Its external interface and what it does is not likely to
drastically change.  On the other hand, "log" is a primary Porcelain
and we would expect constant improvements.

Between the "share more code for reuse" and "keep the flexibility
and clarity for maintainability", it is a subtle balancing act.
Personally I think the current code strikes a good balance by not
going to the extreme, given that "change one (i.e. log) but not the
other" is a very likely pattern for the evolution of these two
commands.

  reply	other threads:[~2013-08-08  6:39 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-07 16:00 Remove old forgotten command: whatchanged Ramkumar Ramachandra
2013-08-07 16:51 ` Stefan Beller
2013-08-07 17:50   ` Junio C Hamano
2013-08-07 21:50     ` Stefan Beller
2013-08-08  6:39       ` Junio C Hamano [this message]
2013-08-08  4:30     ` Ramkumar Ramachandra
2013-08-08 15:03       ` Matthieu Moy
2013-08-08 15:13         ` Ramkumar Ramachandra
2013-08-08 15:24           ` Matthieu Moy
2013-08-08 17:23             ` Junio C Hamano
2013-08-09 20:01               ` [PATCH] whatchanged: document its historical nature Junio C Hamano
2013-08-09 20:14                 ` John Keeping
2013-08-09 20:57                   ` Junio C Hamano
2013-08-12  7:50                     ` John Keeping
2013-08-13 15:56                       ` Junio C Hamano
2013-08-10  7:04                 ` Ramkumar Ramachandra
2013-08-08 17:51         ` Remove old forgotten command: whatchanged Damien Robert
2013-08-08 18:05           ` Ramkumar Ramachandra
2013-08-08 18:06           ` Matthieu Moy
2013-08-08 19:09             ` John Keeping
2013-08-08 19:27             ` Junio C Hamano
2013-08-09  8:29               ` Matthieu Moy
2013-08-09 17:28                 ` Junio C Hamano
2013-08-13  7:58                   ` Matthieu Moy
2013-08-13 16:00                     ` Junio C Hamano
2013-08-08 19:19           ` Junio C Hamano
2013-08-09  0:04             ` Damien Robert
2013-08-09  0:11               ` Junio C Hamano
2013-08-07 18:01 ` Kyle J. McKay
2013-08-07 18:31   ` John Keeping
2013-08-07 18:48     ` Kyle J. McKay

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vpptobsa2.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=artagnon@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=stefanbeller@googlemail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).