* [PATCH] Improve git-prune -n output
@ 2006-10-26 10:38 Andy Parkins
2006-10-26 17:21 ` Junio C Hamano
2006-10-26 17:33 ` Andy Whitcroft
0 siblings, 2 replies; 12+ messages in thread
From: Andy Parkins @ 2006-10-26 10:38 UTC (permalink / raw)
To: git
prune_object() in show_only mode would previously just show the path to the
object that would be deleted. The path the object is stored in shouldn't be
shown to users, they only know about sha1 identifiers so show that instead.
Further, the sha1 alone isn't that useful for examining what is going to be
deleted. This patch also adds the object type to the output, which makes it
easy to pick out, say, the commits and use git-show to display them.
Signed-off-by: Andy Parkins <andyparkins@gmail.com>
---
builtin-prune.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/builtin-prune.c b/builtin-prune.c
index 7290e6d..e3bcf5f 100644
--- a/builtin-prune.c
+++ b/builtin-prune.c
@@ -16,8 +16,13 @@ static struct rev_info revs;
static int prune_object(char *path, const char *filename, const unsigned char *sha1)
{
+ char type[20];
+
if (show_only) {
- printf("would prune %s/%s\n", path, filename);
+ if (sha1_object_info(sha1, type, NULL)) {
+ strcpy( type, "unknown type" );
+ }
+ printf("would prune %s %s\n", sha1_to_hex( sha1 ), type );
return 0;
}
unlink(mkpath("%s/%s", path, filename));
--
1.4.2.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] Improve git-prune -n output
2006-10-26 10:38 [PATCH] Improve git-prune -n output Andy Parkins
@ 2006-10-26 17:21 ` Junio C Hamano
2006-10-26 17:37 ` Andy Parkins
2006-10-27 8:37 ` Andy Parkins
2006-10-26 17:33 ` Andy Whitcroft
1 sibling, 2 replies; 12+ messages in thread
From: Junio C Hamano @ 2006-10-26 17:21 UTC (permalink / raw)
To: Andy Parkins; +Cc: git
Andy Parkins <andyparkins@gmail.com> writes:
> prune_object() in show_only mode would previously just show the path to the
> object that would be deleted. The path the object is stored in shouldn't be
> shown to users, they only know about sha1 identifiers so show that instead.
I do not have an objection to what this does, except I wonder if
somebody's script relies on parsing this output already.
> Further, the sha1 alone isn't that useful for examining what is going to be
> deleted. This patch also adds the object type to the output, which makes it
> easy to pick out, say, the commits and use git-show to display them.
> Signed-off-by: Andy Parkins <andyparkins@gmail.com>
It is customary to have one empty line before the S-o-b: line.
> ---
> builtin-prune.c | 7 ++++++-
> 1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/builtin-prune.c b/builtin-prune.c
> index 7290e6d..e3bcf5f 100644
> --- a/builtin-prune.c
> +++ b/builtin-prune.c
> @@ -16,8 +16,13 @@ static struct rev_info revs;
>
> static int prune_object(char *path, const char *filename, const unsigned char *sha1)
> {
> + char type[20];
> +
> if (show_only) {
> - printf("would prune %s/%s\n", path, filename);
> + if (sha1_object_info(sha1, type, NULL)) {
> + strcpy( type, "unknown type" );
> + }
> + printf("would prune %s %s\n", sha1_to_hex( sha1 ), type );
Style.
- We do not leave blank inside () pairs.
- We do not enclose a single statement in {}.
Perhaps an extra variable of type "char *" that initially point
at type[] but later points at the "unknown" might be more
efficient, but that being on the error path I do not think
saving one strcpy() that way is worth it.
So, just the style and a slight worry that this _might_ break
people's scripts.
Objections?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Improve git-prune -n output
2006-10-26 17:21 ` Junio C Hamano
@ 2006-10-26 17:37 ` Andy Parkins
2006-10-27 8:37 ` Andy Parkins
1 sibling, 0 replies; 12+ messages in thread
From: Andy Parkins @ 2006-10-26 17:37 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
On Thursday 2006, October 26 18:21, Junio C Hamano wrote:
> I do not have an objection to what this does, except I wonder if
> somebody's script relies on parsing this output already.
I did think of that, but the existing output looked pretty non-useful for easy
parsing because of the fact that filenames where being printed instead of
hashes. If I were given free reign I think I'd want to drop the "would
prune" message as well.
> It is customary to have one empty line before the S-o-b: line.
I didn't do it. Git did :-) I'll keep an eye out for it from now on.
Apologies (as usual)
Andy
--
Dr Andrew Parkins, M Eng (Hons), AMIEE
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] Improve git-prune -n output
2006-10-26 17:21 ` Junio C Hamano
2006-10-26 17:37 ` Andy Parkins
@ 2006-10-27 8:37 ` Andy Parkins
2006-10-28 14:00 ` Jakub Narebski
1 sibling, 1 reply; 12+ messages in thread
From: Andy Parkins @ 2006-10-27 8:37 UTC (permalink / raw)
To: git
prune_object() in show_only mode would previously just show the path to the
object that would be deleted. The path the object is stored in shouldn't be
shown to users, they only know about sha1 identifiers so show that instead.
Further, the sha1 alone isn't that useful for examining what is going to be
deleted. This patch also adds the object type to the output, which makes it
easy to pick out, say, the commits and use git-show to display them.
Signed-off-by: Andy Parkins <andyparkins@gmail.com>
---
Fixes issues from...
Junio C Hamano
Do not leave blank inside () pairs.
Do not enclose a single statement in {}.
No longer uses strcpy()
Petr Baudis
"unknown type" is now simply "unknown"
Andy Parkins
No longer outputs "would prune"
builtin-prune.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/builtin-prune.c b/builtin-prune.c
index 7290e6d..b8b2d05 100644
--- a/builtin-prune.c
+++ b/builtin-prune.c
@@ -16,8 +16,14 @@ static struct rev_info revs;
static int prune_object(char *path, const char *filename, const unsigned char
*sha1)
{
+ char buf[20];
+ const char *type;
+
if (show_only) {
- printf("would prune %s/%s\n", path, filename);
+ type = buf;
+ if (sha1_object_info(sha1, type, NULL))
+ type = "unknown";
+ printf("%s %s\n", sha1_to_hex(sha1), type );
return 0;
}
unlink(mkpath("%s/%s", path, filename));
--
1.4.3.3.g5bca6
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] Improve git-prune -n output
2006-10-27 8:37 ` Andy Parkins
@ 2006-10-28 14:00 ` Jakub Narebski
2006-10-28 20:37 ` Junio C Hamano
0 siblings, 1 reply; 12+ messages in thread
From: Jakub Narebski @ 2006-10-28 14:00 UTC (permalink / raw)
To: git
Andy Parkins wrote:
> prune_object() in show_only mode would previously just show the path to the
> object that would be deleted. The path the object is stored in shouldn't be
> shown to users, they only know about sha1 identifiers so show that instead.
This allowed to 'rm -f' [some] of what git-prune -n shows.
Did anybody used that? I don't know...
> Further, the sha1 alone isn't that useful for examining what is going to be
> deleted. This patch also adds the object type to the output, which makes it
> easy to pick out, say, the commits and use git-show to display them.
Well, you can always use git-lost-found. Perhaps it would be better to add
-n option to git-lost-found to not create refs in $GIT_DIR/lost-found/
directory.
Sorry for being late with that suggestion.
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Improve git-prune -n output
2006-10-28 14:00 ` Jakub Narebski
@ 2006-10-28 20:37 ` Junio C Hamano
0 siblings, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2006-10-28 20:37 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
Jakub Narebski <jnareb@gmail.com> writes:
> Andy Parkins wrote:
>
>> prune_object() in show_only mode would previously just show the path to the
>> object that would be deleted. The path the object is stored in shouldn't be
>> shown to users, they only know about sha1 identifiers so show that instead.
>
> This allowed to 'rm -f' [some] of what git-prune -n shows.
>
> Did anybody used that? I don't know...
Yes and probably no because the output did not have enough clues
to help picking which ones to remove. If the command said
something like this:
.git/objetcts/11/feed0... ;# commit 2006-08-28: WIP for "foo"
it might have been a good clue and would have made sense, though.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Improve git-prune -n output
2006-10-26 10:38 [PATCH] Improve git-prune -n output Andy Parkins
2006-10-26 17:21 ` Junio C Hamano
@ 2006-10-26 17:33 ` Andy Whitcroft
2006-10-27 1:45 ` Petr Baudis
2006-10-27 8:19 ` Andy Parkins
1 sibling, 2 replies; 12+ messages in thread
From: Andy Whitcroft @ 2006-10-26 17:33 UTC (permalink / raw)
To: Andy Parkins; +Cc: git
Andy Parkins wrote:
> prune_object() in show_only mode would previously just show the path to the
> object that would be deleted. The path the object is stored in shouldn't be
> shown to users, they only know about sha1 identifiers so show that instead.
>
> Further, the sha1 alone isn't that useful for examining what is going to be
> deleted. This patch also adds the object type to the output, which makes it
> easy to pick out, say, the commits and use git-show to display them.
> Signed-off-by: Andy Parkins <andyparkins@gmail.com>
> ---
> builtin-prune.c | 7 ++++++-
> 1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/builtin-prune.c b/builtin-prune.c
> index 7290e6d..e3bcf5f 100644
> --- a/builtin-prune.c
> +++ b/builtin-prune.c
> @@ -16,8 +16,13 @@ static struct rev_info revs;
>
> static int prune_object(char *path, const char *filename, const unsigned char *sha1)
> {
> + char type[20];
> +
> if (show_only) {
> - printf("would prune %s/%s\n", path, filename);
> + if (sha1_object_info(sha1, type, NULL)) {
> + strcpy( type, "unknown type" );
> + }
> + printf("would prune %s %s\n", sha1_to_hex( sha1 ), type );
> return 0;
> }
> unlink(mkpath("%s/%s", path, filename));
If we are changing the format would it not make more sense to be in the
same order as the tool that lets you use it?
I thought that was git cat-file <type> <commit-ish>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Improve git-prune -n output
2006-10-26 17:33 ` Andy Whitcroft
@ 2006-10-27 1:45 ` Petr Baudis
2006-10-27 8:19 ` Andy Parkins
1 sibling, 0 replies; 12+ messages in thread
From: Petr Baudis @ 2006-10-27 1:45 UTC (permalink / raw)
To: Andy Whitcroft; +Cc: Andy Parkins, junkio, git
Junio wrote:
> - We do not enclose a single statement in {}.
I don't think we need to enforce this rule in particular too rigorously
since in comparison with other style slips, it doesn't put you off by
unusual styling when you are reading the code, its only effect at all is
slight vertical space waste (but it isn't a pure loss anyway, for that
matter).
Dear diary, on Thu, Oct 26, 2006 at 07:33:35PM CEST, I got a letter
where Andy Whitcroft <apw@shadowen.org> said that...
> Andy Parkins wrote:
> > prune_object() in show_only mode would previously just show the path to the
> > object that would be deleted. The path the object is stored in shouldn't be
> > shown to users, they only know about sha1 identifiers so show that instead.
> >
> > Further, the sha1 alone isn't that useful for examining what is going to be
> > deleted. This patch also adds the object type to the output, which makes it
> > easy to pick out, say, the commits and use git-show to display them.
> > Signed-off-by: Andy Parkins <andyparkins@gmail.com>
> > ---
> > builtin-prune.c | 7 ++++++-
> > 1 files changed, 6 insertions(+), 1 deletions(-)
> >
> > diff --git a/builtin-prune.c b/builtin-prune.c
> > index 7290e6d..e3bcf5f 100644
> > --- a/builtin-prune.c
> > +++ b/builtin-prune.c
> > @@ -16,8 +16,13 @@ static struct rev_info revs;
> >
> > static int prune_object(char *path, const char *filename, const unsigned char *sha1)
> > {
> > + char type[20];
> > +
> > if (show_only) {
> > - printf("would prune %s/%s\n", path, filename);
> > + if (sha1_object_info(sha1, type, NULL)) {
> > + strcpy( type, "unknown type" );
> > + }
> > + printf("would prune %s %s\n", sha1_to_hex( sha1 ), type );
> > return 0;
> > }
> > unlink(mkpath("%s/%s", path, filename));
>
> If we are changing the format would it not make more sense to be in the
> same order as the tool that lets you use it?
>
> I thought that was git cat-file <type> <commit-ish>
Seconded and please use just 'unknown' for the type string, so that you
can split the lines on spaces in scripts.
Besides, in what situation do we actually get the unknown output?
Thanks,
--
Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Improve git-prune -n output
2006-10-26 17:33 ` Andy Whitcroft
2006-10-27 1:45 ` Petr Baudis
@ 2006-10-27 8:19 ` Andy Parkins
2006-10-27 9:09 ` Junio C Hamano
1 sibling, 1 reply; 12+ messages in thread
From: Andy Parkins @ 2006-10-27 8:19 UTC (permalink / raw)
To: git
On Thursday 2006 October 26 18:33, Andy Whitcroft wrote:
> If we are changing the format would it not make more sense to be in the
> same order as the tool that lets you use it?
>
> I thought that was git cat-file <type> <commit-ish>
The only problem with that is that the type string is of variable width, while
the hash is fixed; hence <hash> <type> is more visually appealing than <type>
<hash>.
Does it really matter anyway? Some sort of processing would have to happen
before using git-prune output as git-cat-file parameters...
git-prune -n |
while read hash type
do
git-cat-file $type $hash
done
Andy
--
Dr Andy Parkins, M Eng (hons), MIEE
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Improve git-prune -n output
2006-10-27 8:19 ` Andy Parkins
@ 2006-10-27 9:09 ` Junio C Hamano
0 siblings, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2006-10-27 9:09 UTC (permalink / raw)
To: Andy Parkins; +Cc: git
Andy Parkins <andyparkins@gmail.com> writes:
> The only problem with that is that the type string is of
> variable width, while the hash is fixed; hence <hash> <type>
> is more visually appealing than <type> <hash>.
>
> Does it really matter anyway?
I would rather question if the alignment really matters. You
could always printf("%10s %s\n", type, hex) if you really cared,
though.
> ... Some sort of processing would have to happen
> before using git-prune output as git-cat-file parameters...
>
> git-prune -n |
> while read hash type
> do
> git-cat-file $type $hash
> done
which is probably not the way in which the command output is
going to be used. I'd expect a sequence more like:
$ git prune -n
blob deadbeefdeadbeef...
commit deadbeefdeadbeef...
...
The user would not just pipe all of the output to cat-file, but
would pick ones that are meaningful to be cat'ed (commit and
perhaps blob but definitely not tree). And the cue to pick
which ones would likely to be the object type not hash, so in
that sense type-then-hash might be easier to read.
Having said that, a potentially useful application would be "a
backup before prune" which can be done more easily if you use
hash-then-type: "git prune -n | git pack-objects backup"
So while I do not think it matters that much, I slightly prefer
hash-then-type better than type-then-hash.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] Improve git-prune -n output
@ 2006-11-02 11:12 Andy Parkins
2006-11-03 2:40 ` Junio C Hamano
0 siblings, 1 reply; 12+ messages in thread
From: Andy Parkins @ 2006-11-02 11:12 UTC (permalink / raw)
To: git
prune_object() in show_only mode would previously just show the path to the
object that would be deleted. The path the object is stored in shouldn't be
shown to users, they only know about sha1 identifiers so show that instead.
Further, the sha1 alone isn't that useful for examining what is going to be
deleted. This patch also adds the object type to the output, which makes it
easy to pick out, say, the commits and use git-show to display them.
Signed-off-by: Andy Parkins <andyparkins@gmail.com>
---
builtin-prune.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/builtin-prune.c b/builtin-prune.c
index d853902..286a94c 100644
--- a/builtin-prune.c
+++ b/builtin-prune.c
@@ -16,8 +16,14 @@ static struct rev_info revs;
static int prune_object(char *path, const char *filename, const unsigned char *sha1)
{
+ char buf[20];
+ const char *type;
+
if (show_only) {
- printf("would prune %s/%s\n", path, filename);
+ type = buf;
+ if (sha1_object_info(sha1, type, NULL))
+ type = "unknown";
+ printf("%s %s\n", sha1_to_hex(sha1), type );
return 0;
}
unlink(mkpath("%s/%s", path, filename));
--
1.4.3.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] Improve git-prune -n output
2006-11-02 11:12 Andy Parkins
@ 2006-11-03 2:40 ` Junio C Hamano
0 siblings, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2006-11-03 2:40 UTC (permalink / raw)
To: Andy Parkins; +Cc: git
Andy Parkins <andyparkins@gmail.com> writes:
> prune_object() in show_only mode would previously just show the path to the
> object that would be deleted. The path the object is stored in shouldn't be
> shown to users, they only know about sha1 identifiers so show that instead.
>
> Further, the sha1 alone isn't that useful for examining what is going to be
> deleted. This patch also adds the object type to the output, which makes it
> easy to pick out, say, the commits and use git-show to display them.
>
> Signed-off-by: Andy Parkins <andyparkins@gmail.com>
List members and git users,
if you are using "prune -n" output in your scripts, this
change could cause those scripts to break; so please holler.
I am inclined to take this output format change, so if you are
going to holler please do so fast.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2006-11-03 2:45 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-10-26 10:38 [PATCH] Improve git-prune -n output Andy Parkins
2006-10-26 17:21 ` Junio C Hamano
2006-10-26 17:37 ` Andy Parkins
2006-10-27 8:37 ` Andy Parkins
2006-10-28 14:00 ` Jakub Narebski
2006-10-28 20:37 ` Junio C Hamano
2006-10-26 17:33 ` Andy Whitcroft
2006-10-27 1:45 ` Petr Baudis
2006-10-27 8:19 ` Andy Parkins
2006-10-27 9:09 ` Junio C Hamano
-- strict thread matches above, loose matches on Subject: below --
2006-11-02 11:12 Andy Parkins
2006-11-03 2:40 ` Junio C Hamano
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).