[PATCH 0/5] Rework diff options

This patch series cleans up diff output format options.

This makes it possible to use any combination of --raw, -p, --stat and
--summary options and they work as you would expect.

These patches passed all tests but patch 2/5 is quite intrusive...

 b/builtin-diff-files.c  |   10 +-
 b/builtin-diff-index.c  |    4 
 b/builtin-diff-stages.c |    3 
 b/builtin-diff-tree.c   |    3 
 b/builtin-diff.c        |   74 ++++++-----------
 b/builtin-log.c         |   12 +-
 b/combine-diff.c        |   45 +++-------
 b/diff.c                |  207 +++++++++++++++++++++++-------------------------
 b/diff.h                |   29 +++---
 b/git-merge.sh          |    3 
 b/log-tree.c            |   15 ++-
 b/revision.c            |    5 -
 12 files changed, 196 insertions(+), 214 deletions(-)

-- 
http://onion.dynserv.net/~timo/

Hi,

On Sat, 24 Jun 2006, Timo Hirvonen wrote:

This patch series cleans up diff output format options.

Very good.

Although I understand that to convert all users to the new convention, it 
is sensible to rename the constants, I think it is not good to change 
something as DIFF_FORMAT_RAW to OUTPUT_FMT_RAW in the resulting patch.

IMHO it is an unnecessary change, and accounts for a lot of the diffstat.

Ciao,
Dscho

Johannes Schindelin [off-list ref] wrote:

Hi,

On Sat, 24 Jun 2006, Timo Hirvonen wrote:

quoted

This patch series cleans up diff output format options.

Very good.

Although I understand that to convert all users to the new convention, it 
is sensible to rename the constants, I think it is not good to change 
something as DIFF_FORMAT_RAW to OUTPUT_FMT_RAW in the resulting patch.

IMHO it is an unnecessary change, and accounts for a lot of the diffstat.

I did it because you can't have many DIFF_FORMAT_* options at the same
time but OUTPUT_FMT_* can be combined.

-- 
http://onion.dynserv.net/~timo/

On Sat, 24 Jun 2006, Timo Hirvonen wrote:

This patch series cleans up diff output format options.

This makes it possible to use any combination of --raw, -p, --stat and
--summary options and they work as you would expect.

Looks good to me. I'll be very happy never having to remember the option 
(or type) --patch-with-stat ever again. Doing just "-p --stat" is just 
_so_ much better.

		Linus

Hi,

On Sat, 24 Jun 2006, Timo Hirvonen wrote:

Johannes Schindelin [off-list ref] wrote:

quoted

Although I understand that to convert all users to the new convention, it 
is sensible to rename the constants, I think it is not good to change 
something as DIFF_FORMAT_RAW to OUTPUT_FMT_RAW in the resulting patch.

Note that I understand this for the purpose of not forgetting to change 
things over to "|=" and "&": the compiler will warn you about that now.

But after it compiles, you can change the names back to reduce patch size 
and to avoid confusing of dumb people like me.

quoted

IMHO it is an unnecessary change, and accounts for a lot of the diffstat.

I did it because you can't have many DIFF_FORMAT_* options at the same
time but OUTPUT_FMT_* can be combined.

But you just renamed them! The name alone does not say "you cannot combine 
them".

-- snip --

@@ -150,15 +162,6 @@ #define COMMON_DIFF_OPTIONS_HELP \
 "                show all files diff when -S is used and hit is found.\n"
 
 extern int diff_queue_is_empty(void);
-
-#define DIFF_FORMAT_RAW                1
-#define DIFF_FORMAT_PATCH      2
-#define DIFF_FORMAT_NO_OUTPUT  3
-#define DIFF_FORMAT_NAME       4
-#define DIFF_FORMAT_NAME_STATUS        5
-#define DIFF_FORMAT_DIFFSTAT   6
-#define DIFF_FORMAT_CHECKDIFF  7
-
-- snap --

You also sneak in some other things, such as renaming output_format to 
output_fmt in struct diff_options, making a function static, and expanding 
a "(a ? b : c)", without accounting for it in the commit message.

Ciao,
Dscho