Re: [PATCH v3 07/10] builtin/replace: teach listing using short, medium or full formats

2013-12-28 Thread Christian Couder 
From: Junio C Hamano gits...@pobox.com

 Christian Couder christian.cou...@gmail.com writes: 

 Ok, so would you prefer the following:

 - NAME_ONLY_REPLACE_FMT and --format=name_only instead of
 SHORT_REPLACE_FMT and --format=short

 - NAME_AND_VALUE_REPLACE_FMT and --format=name_and_value instead of
 MEDIUM_REPLACE_FMT and --format=medium

 - DEBUG_REPLACE_FMT and --format=debug instead of FULL _REPLACE_FMT
 and --format=full
 
 The end-user facing names are probably fine with short, medium,
 full, as long as what they show are clearly explained in the
 end-user documentation (patch 10/10 covers this).

Ok, I will try to improve on that.

 I have a hunch that we may later regret full when somebody wants
 to add even fuller information, though. It might be better spelled
 long instead;

Ok, I will use long instead.

 I'd rather see REPLACE_FMT_ as a prefix, not suffix.  Do we use
 common suffix for enum values elsewhere?

I don't see common suffix, but we have the following enums about
formats:

* in builtin/commit.c:

static enum status_format {
STATUS_FORMAT_NONE = 0,
STATUS_FORMAT_LONG,
STATUS_FORMAT_SHORT,
STATUS_FORMAT_PORCELAIN,

STATUS_FORMAT_UNSPECIFIED
} status_format = STATUS_FORMAT_UNSPECIFIED;

* in builtin/help.c:

enum help_format {
HELP_FORMAT_NONE,
HELP_FORMAT_MAN,
HELP_FORMAT_INFO,
HELP_FORMAT_WEB
};

* in commit.h

enum cmit_fmt {
CMIT_FMT_RAW,
CMIT_FMT_MEDIUM,
CMIT_FMT_DEFAULT = CMIT_FMT_MEDIUM,
CMIT_FMT_SHORT,
CMIT_FMT_FULL,
CMIT_FMT_FULLER,
CMIT_FMT_ONELINE,
CMIT_FMT_EMAIL,
CMIT_FMT_USERFORMAT,

CMIT_FMT_UNSPECIFIED
};

To conform to the above and what you suggest, I will send a new series
using the following:

enum replace_format {
  REPLACE_FORMAT_SHORT,
  REPLACE_FORMAT_MEDIUM,
  REPLACE_FORMAT_LONG
};

Thanks,
Christian.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 07/10] builtin/replace: teach listing using short, medium or full formats

2013-12-26 Thread Junio C Hamano 
Christian Couder christian.cou...@gmail.com writes:

 On Thu, Dec 19, 2013 at 7:58 PM, Junio C Hamano gits...@pobox.com wrote:
 Christian Couder christian.cou...@gmail.com writes:

 I think this last one might be useful for people replacing objects
 with objects that have another type.

 ... which IIUC is strongly discouraged---didn't you have to tighten
 it recently?

 And that makes it useful primarily for debugging unusual
 situations.

 Ok, so would you prefer the following:

 - NAME_ONLY_REPLACE_FMT and --format=name_only instead of
 SHORT_REPLACE_FMT and --format=short

 - NAME_AND_VALUE_REPLACE_FMT and --format=name_and_value instead of
 MEDIUM_REPLACE_FMT and --format=medium

 - DEBUG_REPLACE_FMT and --format=debug instead of FULL _REPLACE_FMT
 and --format=full

The end-user facing names are probably fine with short, medium,
full, as long as what they show are clearly explained in the
end-user documentation (patch 10/10 covers this).

I have a hunch that we may later regret full when somebody wants
to add even fuller information, though. It might be better spelled
long instead;

I'd rather see REPLACE_FMT_ as a prefix, not suffix.  Do we use
common suffix for enum values elsewhere?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 07/10] builtin/replace: teach listing using short, medium or full formats

2013-12-21 Thread Christian Couder 
On Thu, Dec 19, 2013 at 7:58 PM, Junio C Hamano gits...@pobox.com wrote:
 Christian Couder christian.cou...@gmail.com writes:

 I think this last one might be useful for people replacing objects
 with objects that have another type.

 ... which IIUC is strongly discouraged---didn't you have to tighten
 it recently?

 And that makes it useful primarily for debugging unusual
 situations.

Ok, so would you prefer the following:

- NAME_ONLY_REPLACE_FMT and --format=name_only instead of
SHORT_REPLACE_FMT and --format=short

- NAME_AND_VALUE_REPLACE_FMT and --format=name_and_value instead of
MEDIUM_REPLACE_FMT and --format=medium

- DEBUG_REPLACE_FMT and --format=debug instead of FULL _REPLACE_FMT
and --format=full

?

Thanks,
Christian.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 07/10] builtin/replace: teach listing using short, medium or full formats

2013-12-19 Thread Christian Couder 
On Wed, Dec 18, 2013 at 6:37 PM, Junio C Hamano gits...@pobox.com wrote:
 Christian Couder christian.cou...@gmail.com writes:

 On Wed, Dec 18, 2013 at 1:37 PM, Karsten Blees karsten.bl...@gmail.com 
 wrote:
 Am 11.12.2013 08:46, schrieb Christian Couder:
 +enum repl_fmt { SHORT, MEDIUM, FULL };

 SHORT is predefined on Windows, could you choose another name?

 Ok, I will change to:

 enum repl_fmt { SHORT_FMT, MEDIUM_FMT, FULL_FMT };

 What are these for in the first place?  Your SHORT conflicting
 with something totally unrelated is a sign that you should be naming
 them in a way that is more specific to your use.  SHORT_FMT is still
 not specific enough to tell what they are for.  With SHOW_REPLACE_
 prefix, perhaps?  Also perhaps give characterization better than
 their output lengths?

I am ok with SHORT_REPLACE_FMT and so on.

 My quick read of show_reference() tells me that they are name
 only, name and value, and something else that does not seem
 very useful unless you are debugging.

Yeah, SHORT_REPLACE_FMT is name only which means something like:

$ git replace --format=short
14ac020163ea60a9d683ce68e36c946f31ecc856
4b48deed3a433909bfd6b6ab3d4b91348b6af464
5c37393794868bc8e708cccd7c9d9aaa7a5e53cb
a3fb2e1845a1aaf129b7975048973414dc172173
e25dc7954f0832d962347872884aab2dffb426c5

MEDIUM_REPLACE_FMT is name and value, like this:

$ git replace --format=medium
14ac020163ea60a9d683ce68e36c946f31ecc856 -
4b48deed3a433909bfd6b6ab3d4b91348b6af464
4b48deed3a433909bfd6b6ab3d4b91348b6af464 -
feae347d8510cfba5eb8c8ac80056777b07c2528
5c37393794868bc8e708cccd7c9d9aaa7a5e53cb -
14ac020163ea60a9d683ce68e36c946f31ecc856
a3fb2e1845a1aaf129b7975048973414dc172173 -
9af2a15082b7c95982473e32f3376558c149a7e7
e25dc7954f0832d962347872884aab2dffb426c5 -
00ad688edb1a79423184992de45a5f0322c8bdf5

and FULL _REPLACE_FMT is name with type and value with type, like this:

$ git replace --format=full
14ac020163ea60a9d683ce68e36c946f31ecc856 (commit) -
4b48deed3a433909bfd6b6ab3d4b91348b6af464 (blob)
4b48deed3a433909bfd6b6ab3d4b91348b6af464 (blob) -
feae347d8510cfba5eb8c8ac80056777b07c2528 (blob)
5c37393794868bc8e708cccd7c9d9aaa7a5e53cb (tree) -
14ac020163ea60a9d683ce68e36c946f31ecc856 (commit)
a3fb2e1845a1aaf129b7975048973414dc172173 (commit) -
9af2a15082b7c95982473e32f3376558c149a7e7 (commit)
e25dc7954f0832d962347872884aab2dffb426c5 (tag) -
00ad688edb1a79423184992de45a5f0322c8bdf5 (commit)

I think this last one might be useful for people replacing objects
with objects that have another type.
And as we let people do that using --force, it could be useful for
them to have a way to easily see what they have done.

Thanks,
Christian.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 07/10] builtin/replace: teach listing using short, medium or full formats

2013-12-19 Thread Junio C Hamano 
Christian Couder christian.cou...@gmail.com writes:

 I think this last one might be useful for people replacing objects
 with objects that have another type.

... which IIUC is strongly discouraged---didn't you have to tighten
it recently?

And that makes it useful primarily for debugging unusual
situations.

 And as we let people do that using --force, it could be useful for
 them to have a way to easily see what they have done.

 Thanks,
 Christian.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 07/10] builtin/replace: teach listing using short, medium or full formats

2013-12-18 Thread Karsten Blees 
Am 11.12.2013 08:46, schrieb Christian Couder:
 +enum repl_fmt { SHORT, MEDIUM, FULL };

SHORT is predefined on Windows, could you choose another name?

MinGW:

builtin/replace.c:23: error: 'SHORT' redeclared as different kind of symbol
c:\git\msysgit\mingw\bin\../lib/gcc/mingw32/4.4.0/../../../../include/winnt.h:78:
 note: previous declaration of 'SHORT'
was here
make: *** [builtin/replace.o] Error 1


MSVC:

CC builtin/replace.o
replace.c
builtin/replace.c(23) : error C2365: SHORT: Erneute Definition; vorherige 
Definition war Typedef.
C:\Program Files\Microsoft SDKs\Windows\v7.1\include\winnt.h(332): 
Siehe Deklaration von 'SHORT'
builtin/replace.c(23) : error C2086: 'repl_fmt SHORT': Neudefinition
builtin/replace.c(23): Siehe Deklaration von 'SHORT'
builtin/replace.c(36) : error C2275: 'SHORT': Ungültige Verwendung dieses Typs 
als Ausdruck
C:\Program Files\Microsoft SDKs\Windows\v7.1\include\winnt.h(332): 
Siehe Deklaration von 'SHORT'
builtin/replace.c(67) : error C2275: 'SHORT': Ungültige Verwendung dieses Typs 
als Ausdruck
C:\Program Files\Microsoft SDKs\Windows\v7.1\include\winnt.h(332): 
Siehe Deklaration von 'SHORT'
builtin/replace.c(173) : warning C4090: 'Initialisierung': Unterschiedliche 
'const'-Bezeichner
make: *** [builtin/replace.o] Error 1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 07/10] builtin/replace: teach listing using short, medium or full formats

2013-12-18 Thread Christian Couder 
On Wed, Dec 18, 2013 at 1:37 PM, Karsten Blees karsten.bl...@gmail.com wrote:
 Am 11.12.2013 08:46, schrieb Christian Couder:
 +enum repl_fmt { SHORT, MEDIUM, FULL };

 SHORT is predefined on Windows, could you choose another name?

Ok, I will change to:

enum repl_fmt { SHORT_FMT, MEDIUM_FMT, FULL_FMT };

Thanks,
Christian.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 07/10] builtin/replace: teach listing using short, medium or full formats

2013-12-10 Thread Christian Couder 
By default when listing replace refs, only the sha1 of the
replaced objects are shown.

In many cases, it is much nicer to be able to list all the
sha1 of the replaced objects along with the sha1 of the
replacment objects.

And in other cases it might be interesting to also show the
types of the replaced and replacement objects.

This patch introduce a new --format=fmt option where
fmt can be any of the following:

'short': this is the same as when no --format
option is used, that is only the sha1 of
the replaced objects are shown
'medium': this also lists the sha1 of the
replacement objects
'full': this shows the sha1 and the type of both
the replaced and the replacement objects

Some documentation and some tests will follow.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 builtin/replace.c | 61 ---
 1 file changed, 54 insertions(+), 7 deletions(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index b1bd3ef..9f3619a 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -16,27 +16,65 @@
 static const char * const git_replace_usage[] = {
N_(git replace [-f] object replacement),
N_(git replace -d object...),
-   N_(git replace -l [pattern]),
+   N_(git replace [--format=format] [-l [pattern]]),
NULL
 };
 
+enum repl_fmt { SHORT, MEDIUM, FULL };
+
+struct show_data {
+   const char *pattern;
+   enum repl_fmt fmt;
+};
+
 static int show_reference(const char *refname, const unsigned char *sha1,
  int flag, void *cb_data)
 {
-   const char *pattern = cb_data;
+   struct show_data *data = cb_data;
+
+   if (!fnmatch(data-pattern, refname, 0)) {
+   if (data-fmt == SHORT)
+   printf(%s\n, refname);
+   else if (data-fmt == MEDIUM)
+   printf(%s - %s\n, refname, sha1_to_hex(sha1));
+   else { /* data-fmt == FULL */
+   unsigned char object[20];
+   enum object_type obj_type, repl_type;
 
-   if (!fnmatch(pattern, refname, 0))
-   printf(%s\n, refname);
+   if (get_sha1(refname, object))
+   return error(Failed to resolve '%s' as a valid 
ref., refname);
+
+   obj_type = sha1_object_info(object, NULL);
+   repl_type = sha1_object_info(sha1, NULL);
+
+   printf(%s (%s) - %s (%s)\n, refname, 
typename(obj_type),
+  sha1_to_hex(sha1), typename(repl_type));
+   }
+   }
 
return 0;
 }
 
-static int list_replace_refs(const char *pattern)
+static int list_replace_refs(const char *pattern, const char *format)
 {
+   struct show_data data;
+
if (pattern == NULL)
pattern = *;
+   data.pattern = pattern;
+
+   if (format == NULL || *format == '\0' || !strcmp(format, short))
+   data.fmt = SHORT;
+   else if (!strcmp(format, medium))
+   data.fmt = MEDIUM;
+   else if (!strcmp(format, full))
+   data.fmt = FULL;
+   else
+   die(invalid replace format '%s'\n
+   valid formats are 'short', 'medium' and 'full'\n,
+   format);
 
-   for_each_replace_ref(show_reference, (void *) pattern);
+   for_each_replace_ref(show_reference, (void *) data);
 
return 0;
 }
@@ -127,10 +165,12 @@ static int replace_object(const char *object_ref, const 
char *replace_ref,
 int cmd_replace(int argc, const char **argv, const char *prefix)
 {
int list = 0, delete = 0, force = 0;
+   const char *format = NULL;
struct option options[] = {
OPT_BOOL('l', list, list, N_(list replace refs)),
OPT_BOOL('d', delete, delete, N_(delete replace refs)),
OPT_BOOL('f', force, force, N_(replace the ref if it 
exists)),
+   OPT_STRING(0, format, format, N_(format), N_(use this 
format)),
OPT_END()
};
 
@@ -140,6 +180,10 @@ int cmd_replace(int argc, const char **argv, const char 
*prefix)
usage_msg_opt(-l and -d cannot be used together,
  git_replace_usage, options);
 
+   if (format  delete)
+   usage_msg_opt(--format and -d cannot be used together,
+ git_replace_usage, options);
+
if (force  (list || delete))
usage_msg_opt(-f cannot be used with -d or -l,
  git_replace_usage, options);
@@ -157,6 +201,9 @@ int cmd_replace(int argc, const char **argv, const char 
*prefix)
if (argc != 2)
usage_msg_opt(bad number of arguments,
  git_replace_usage, options);
+   if (format)
+