* git fsck not identifying corrupted packs
From: Sergio Callegari @ 2009-10-19 7:56 UTC (permalink / raw)
To: git
Hi,
I have a pack that contains a corrupted object.
It is an old corrupted repo that I have conserved.
As expected, git gc cries out loud about it.
It indicates an inflate error (data stream error with incorrect data check),
and then the impossibility to read an object from a certain offset in the pack.
However, git fsck does not complain at all about the repo.
I guess that for speed reasons, git fsck does not try to inflate the objects.
Is there a means to have fsck to a truly full check on the sanity of a repo?
This both on git 1.6.5.1 and 1.6.4.2.
Thanks
Sergio Callegari
^ permalink raw reply
* [PATCH v2 3/3] Teach --wrap to only indent without wrapping
From: Junio C Hamano @ 2009-10-19 7:30 UTC (permalink / raw)
To: git
In-Reply-To: <1255937414-10043-1-git-send-email-gitster@pobox.com>
When a zero or negative width is given to "shortlog -w<width>,<in1>,<in2>"
and --format=%[wrap(w,in1,in2)...%], just indent the text by in1 without
wrapping.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
utf8.c | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/utf8.c b/utf8.c
index da99669..5c18f0c 100644
--- a/utf8.c
+++ b/utf8.c
@@ -310,6 +310,19 @@ int strbuf_add_wrapped_text(struct strbuf *buf,
int w = indent, assume_utf8 = is_utf8(text);
const char *bol = text, *space = NULL;
+ if (width <= 0) {
+ /* just indent */
+ while (*text) {
+ const char *eol = strchrnul(text, '\n');
+ if (*eol == '\n')
+ eol++;
+ print_spaces(buf, indent);
+ strbuf_write(buf, text, eol-text);
+ text = eol;
+ }
+ return 1;
+ }
+
if (indent < 0) {
w = -indent;
space = text;
--
1.6.5.1.95.g09fbd
^ permalink raw reply related
* [PATCH v2 2/3] Add %[wrap(width,in1,in2)<<any-string>>%] implementation
From: Junio C Hamano @ 2009-10-19 7:30 UTC (permalink / raw)
To: git
In-Reply-To: <1255937414-10043-1-git-send-email-gitster@pobox.com>
This uses the strbuf_nested_expand() mechanism introduced earlier to
demonstrate how to implement a nested string function, by plugging Dscho's
implementation of strbuf_add_wrapped_text().
The log is much more pleasant to read with
$ git log --format='commit %H%nAuthor: %an%n%n%[wrap(66,4,4)%s%n%n%b%]'
in some repositories (e.g. git-svn conversion of rockbox).
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Renamed w() to wrap() and actually plugged Dscho's strbuf_add_wrapped_text().
pretty.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 76 insertions(+), 0 deletions(-)
diff --git a/pretty.c b/pretty.c
index 126be56..bc74c25 100644
--- a/pretty.c
+++ b/pretty.c
@@ -595,6 +595,72 @@ static void format_decoration(struct strbuf *sb, const struct commit *commit)
strbuf_addch(sb, ')');
}
+typedef int (*string_fmt_fn)(struct strbuf *sb, const char *placeholder, void *context);
+static ssize_t format_commit_item(struct strbuf *, const char *, void *);
+
+static int wrap_fn(struct strbuf *sb, const char *placeholder, void *context)
+{
+ const char *template = placeholder;
+ char *endptr, *nested;
+ long width = 0, indent1 = 0, indent2 = 0;
+
+ width = strtol(template, &endptr, 10);
+ if (*endptr == ',') {
+ template = endptr + 1;
+ indent1 = strtol(template, &endptr, 10);
+ if (*endptr == ',') {
+ template = endptr + 1;
+ indent2 = strtol(template, &endptr, 10);
+ }
+ }
+ if (*endptr++ != ')')
+ return 0;
+
+ template = endptr;
+ strbuf_nested_expand(sb, &template, format_commit_item, context);
+ if (*template++ != ']')
+ return 0;
+
+ nested = strbuf_detach(sb, NULL);
+ strbuf_add_wrapped_text(sb, nested, indent1, indent2, width);
+ free(nested);
+
+ return template - placeholder;
+}
+
+static struct {
+ const char *name;
+ string_fmt_fn fn;
+} format_fn_list[] = {
+ { "wrap(", wrap_fn }
+};
+
+static size_t format_fn(struct strbuf *sb, const char *placeholder,
+ void *context)
+{
+ const char *template = placeholder;
+ size_t consumed;
+ struct strbuf substr = STRBUF_INIT;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(format_fn_list); i++)
+ if (!prefixcmp(template, format_fn_list[i].name))
+ break;
+ if (ARRAY_SIZE(format_fn_list) <= i)
+ return 0;
+ template += strlen(format_fn_list[i].name);
+ consumed = format_fn_list[i].fn(&substr, template, context);
+ if (!consumed) {
+ strbuf_release(&substr);
+ return 0;
+ }
+
+ strbuf_add(sb, substr.buf, substr.len);
+ template += consumed;
+ strbuf_release(&substr);
+ return template - placeholder;
+}
+
static ssize_t format_commit_item(struct strbuf *sb, const char *placeholder,
void *context)
{
@@ -603,9 +669,19 @@ static ssize_t format_commit_item(struct strbuf *sb, const char *placeholder,
const char *msg = commit->buffer;
struct commit_list *p;
int h1, h2;
+ ssize_t nested;
/* these are independent of the commit */
switch (placeholder[0]) {
+ case ']':
+ return -1;
+ case '[':
+ /*
+ * %[func(arg...) string %]: we consumed the opening '['
+ * and the callee consumed up to the closing '%]'.
+ */
+ nested = format_fn(sb, placeholder + 1, context);
+ return nested ? 1 + nested : 0;
case 'C':
if (placeholder[1] == '(') {
const char *end = strchr(placeholder + 2, ')');
^ permalink raw reply related
* [PATCH v2 1/3] strbuf_nested_expand(): allow expansion to interrupt in the middle
From: Junio C Hamano @ 2009-10-19 7:30 UTC (permalink / raw)
To: git
In-Reply-To: <1255937414-10043-1-git-send-email-gitster@pobox.com>
This itself does not do a "nested" expansion, but it paves a way for
supporting an extended syntax to express a function that works on an
expanded substring, e.g. %[function(param...)expanded-string%], by
allowing the callback function to tell where the argument to the function
ends.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Dropped sloppy casts, as suggested by Dscho.
pretty.c | 4 ++--
strbuf.c | 29 ++++++++++++++++++++++-------
strbuf.h | 5 +++--
3 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/pretty.c b/pretty.c
index 587101f..126be56 100644
--- a/pretty.c
+++ b/pretty.c
@@ -595,8 +595,8 @@ static void format_decoration(struct strbuf *sb, const struct commit *commit)
strbuf_addch(sb, ')');
}
-static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
- void *context)
+static ssize_t format_commit_item(struct strbuf *sb, const char *placeholder,
+ void *context)
{
struct format_commit_context *c = context;
const struct commit *commit = c->commit;
diff --git a/strbuf.c b/strbuf.c
index a6153dc..af96155 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -214,29 +214,44 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
strbuf_setlen(sb, sb->len + len);
}
-void strbuf_expand(struct strbuf *sb, const char *format, expand_fn_t fn,
- void *context)
+void strbuf_nested_expand(struct strbuf *sb, const char **format_p,
+ expand_fn_t fn, void *context)
{
+ const char *format = *format_p;
for (;;) {
const char *percent;
- size_t consumed;
+ ssize_t consumed;
percent = strchrnul(format, '%');
strbuf_add(sb, format, percent - format);
+ format = percent;
if (!*percent)
break;
- format = percent + 1;
+ format++;
consumed = fn(sb, format, context);
- if (consumed)
+ if (consumed < 0)
+ break;
+ else if (consumed)
format += consumed;
else
strbuf_addch(sb, '%');
}
+ *format_p = format;
+}
+
+void strbuf_expand(struct strbuf *sb, const char *o_format, expand_fn_t fn,
+ void *context)
+{
+ const char *format = o_format;
+ strbuf_nested_expand(sb, &format, fn, context);
+ if (*format)
+ die("format error: negative return from expand function: %s",
+ o_format);
}
-size_t strbuf_expand_dict_cb(struct strbuf *sb, const char *placeholder,
- void *context)
+ssize_t strbuf_expand_dict_cb(struct strbuf *sb, const char *placeholder,
+ void *context)
{
struct strbuf_expand_dict_entry *e = context;
size_t len;
diff --git a/strbuf.h b/strbuf.h
index d05e056..256d615 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -109,13 +109,14 @@ static inline void strbuf_addbuf(struct strbuf *sb, const struct strbuf *sb2) {
}
extern void strbuf_adddup(struct strbuf *sb, size_t pos, size_t len);
-typedef size_t (*expand_fn_t) (struct strbuf *sb, const char *placeholder, void *context);
+typedef ssize_t (*expand_fn_t)(struct strbuf *sb, const char *placeholder, void *context);
extern void strbuf_expand(struct strbuf *sb, const char *format, expand_fn_t fn, void *context);
+extern void strbuf_nested_expand(struct strbuf *sb, const char **format_p, expand_fn_t fn, void *context);
struct strbuf_expand_dict_entry {
const char *placeholder;
const char *value;
};
-extern size_t strbuf_expand_dict_cb(struct strbuf *sb, const char *placeholder, void *context);
+extern ssize_t strbuf_expand_dict_cb(struct strbuf *sb, const char *placeholder, void *context);
__attribute__((format(printf,2,3)))
extern void strbuf_addf(struct strbuf *sb, const char *fmt, ...);
--
1.6.5.1.95.g09fbd
^ permalink raw reply related
* [PATCH v2 0/3] nested expansion
From: Junio C Hamano @ 2009-10-19 7:30 UTC (permalink / raw)
To: git
This is a re-roll of the earlier "general syntax for applying functions to
pretty-print format output" series, rebased on two patches from Dscho to
implement strbuf_add_wrapped_text(). In other words, this will apply on
top of b4d784c (Add strbuf_add_wrapped_text() to utf8.[ch], 2008-11-10).
The first one is almost the same, except that it drops the unnecessary
cast (pointed out by Dscho) and instead changes the type of return value
from expand callback to ssize_t.
The second one has two changes:
1. w() is renamed to wrap();
2. it actually rewraps the text by using Dscho's strbuf_add_wrapped_text().
The third one is new. It teaches wrap() to just indent without wrapping
when the width parameter is zero or negative.
Junio C Hamano (3):
strbuf_nested_expand(): allow expansion to interrupt in the middle
Add %[wrap(width,in1,in2)<<any-string>>%] implementation
Teach --wrap to only indent without wrapping
pretty.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
strbuf.c | 29 +++++++++++++++++-----
strbuf.h | 5 ++-
utf8.c | 13 ++++++++++
4 files changed, 115 insertions(+), 11 deletions(-)
^ permalink raw reply
* Re: [PATCH] Use "--no-" prefix to switch off some of checkout dwimmery
From: Junio C Hamano @ 2009-10-19 7:25 UTC (permalink / raw)
To: Alex Riesen; +Cc: Junio C Hamano, git, Johannes Schindelin, Jay Soffian
In-Reply-To: <81b0412b0910190017o2e6dfd47v868517404d362843@mail.gmail.com>
Alex Riesen <raa.lkml@gmail.com> writes:
> BTW, why is the option an ...INT? Where a future extension planned?
No, in my original I wanted to default to 1 and an option to set it to
zero, only because I did not want a variable with negative name.
^ permalink raw reply
* Re: [PATCH] Use "--no-" prefix to switch off some of checkout dwimmery
From: Alex Riesen @ 2009-10-19 7:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Schindelin, Jay Soffian
In-Reply-To: <7vhbtv7vsr.fsf@alter.siamese.dyndns.org>
On Mon, Oct 19, 2009 at 08:16, Junio C Hamano <gitster@pobox.com> wrote:
> Alex Riesen <raa.lkml@gmail.com> writes:
>
>> On Mon, Oct 19, 2009 at 08:07, Alex Riesen <raa.lkml@gmail.com> wrote:
>>> On Mon, Oct 19, 2009 at 00:49, Junio C Hamano <gitster@pobox.com> wrote:
>>>> Alex Riesen <raa.lkml@gmail.com> writes:
>>>>> + OPT_SET_INT(0, "dwim", &dwim_new_local_branch,
>>>>> + "Guess local branch from remote reference (default)", 0),
>>>>
>>>> Humph, how does SET_INT know to set it to 1 with --dwim and set it to 0
>>>> with --no-dwim?
>>>
>>> It seems to do, though (I checked before sending).
>>>
>>
>> Right, just looked at the parse-options: it is defined for all types.
>>
>> parse-options.c +/get_value
>>
>> const int unset = flags & OPT_UNSET;
>> ...
>> case OPTION_SET_INT:
>> *(int *)opt->value = unset ? 0 : opt->defval;
>> return 0;
>>
>> Very useful.
>
> Ah, did you mean to change the default value to 1 as well?
>
Err... yes. I (wrongly) assumed that the current value in the
storage is the default. Now, having looked at struct option
I see that It isn't (and the default is in defval).
BTW, why is the option an ...INT? Where a future extension planned?
^ permalink raw reply
* Re: [PATCH] git add -e documentation: rephrase note
From: Junio C Hamano @ 2009-10-19 7:07 UTC (permalink / raw)
To: Jeff King; +Cc: Miklos Vajna, git
In-Reply-To: <20091019063446.GA1457@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> > +* remove addition lines (don't stage the line)
>>
>> This is more like "don't add the line", isn't it? Also if this "+" line
>> has corresponding "-" line (i.e. it is a "rewrite to this" line), removal
>> of such a line would mean "instead of rewriting, remove it".
>
> I was trying not to use "add" because we are already talking about
> addition and removal in the patch itself,
Ah, I wasn't saying "add" is more kosher than "stage" by the above.
By "don't add", I meant that the user is saying "I inserted a new line to
the file, but I actually did not want to add that line to the file for the
next commit." In other words, I was more focusing on the act of inserting
the line to the contents, not on staging the change to the index.
> ... So I am not sure I agree that using "add" is any
> better than "stage", but I don't feel that strongly about it.
That is Ok; the comment was not about stage vs add.
> But beyond that, yes, you are right that removing a "+" line may have a
> different conceptual meaning to the user depending on the surrounding
> text. I wonder if such a "check-list" document really makes much sense,
> given that using "-e" at all means you need to understand the patch
> format and what makes sense (i.e., anybody who understands 'patch' knows
> that you can't just delete context lines and expect it to apply).
Yeah, that is really what I wanted people who are in this discussion to
eventually realize ;-)
> Yeah, again, this comes down to understanding what you are doing:
^ permalink raw reply
* [ANNOUNCE] TortoiseGit 1.2.1.0 Release
From: Frank Li @ 2009-10-19 6:58 UTC (permalink / raw)
To: git, tortoisegit-dev, tortoisegit-users, tortoisegit-announce,
tortoisegit
Download:
http://tortoisegit.googlecode.com/files/TortoiseGit-1.1.1.0-32bit.msi
http://tortoisegit.googlecode.com/files/TortoiseGit-1.1.1.0-64bit.msi
= Release 1.1.1.0 =
== Features ==
* Improve Rebase Dialog
Allow lanuch new rebase dialog again after finish rebase dialog
Disable "force rebase" checkbox during rebase.
* Git SVN
Append svn:ignore settings to the default git exclude file Add
shell extension command to import svn ignore settings.
Need press "Shift" key to show "import svn ignore" command.
* Drag-drop copy\move support
File only
* Add paste command at shell extension
Copy and paste file is okay. But there are problems when including
directory.
Cut and paste working
* Update notepad2 to 4.022
* Sync Dialog
Ability to sync submodules in TGit sync dialog
* Statics
Sort commits by dates before processed by StatGraphDlg
* Log Dialog
Show <No branch> replace ref error message at log dialog
* Add Check software updater support.
== Bug Fix ==
* Fixed issue #185. "Can't find Super-project" when pathname include space.
* Fixed issue #190: Access violation in Blame and wrong path name
when root dir is git repository
* Fixed issue #180: Create patch serial doesn't work when there is
"\" at end of path
* Fixed issue #173: SVN Rebase does not work The correct handle below
case git config svn-remote.svn.fetch
myproject/trunk:refs/remotes/trunk
* Fixed issue #169: Force rebase checkbox is fixed
* Fixed issue #163: Conflict "theirs" and "mine" are reversed during a rebase
* Fixed issue #165: Incorect path to Notepad2
* Fixed issue #158: Rebase can act on the wrong branch
^ permalink raw reply
* Re: Unapplied patches reminder
From: Jeff King @ 2009-10-19 6:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, git
In-Reply-To: <7v63aci8id.fsf@alter.siamese.dyndns.org>
On Sun, Oct 18, 2009 at 04:31:54PM -0700, Junio C Hamano wrote:
> > From: Jeff King <peff@peff.net>
> > Subject: Re: [BUG?] git-cvsimport: path to cvspsfile
> > Date: Wed, 23 Sep 2009 15:14:29 -0400
> > Message-ID: <20090923191428.GA30104@coredump.intra.peff.net>
> >
> > Bug. The script does a chdir() and then looks at the cvspsfile later. I
> > think "-A" would have the same problem. Here is a totally untested patch
> > to address the issue. Johannes, will this is_absolute_path actually work
> > on Windows? I think The Right Way would be to use
> > File::Spec::file_name_is_absolute, but I haven't checked whether that is
> > part of core perl and if so, which version it appeared in.
>
> My understanding of this is that it is a typical "how about this" that is
> still waiting for a follow-up discussion that will result in an eventual
> solution.
Yep. I got comments from JSixt, but I never got around to re-rolling.
Here it is, though still only lightly tested by me (happily, I have not
had to touch CVS for a few years).
-- >8 --
Subject: [PATCH] cvsimport: fix relative argument filenames
One of the first things that cvsimport does is chdir to the
newly created git repo. This means that any filenames given
to us on the command line will be looked up relative to the
git repo directory. This is probably not what the user
expects, so let's remember and prepend the original
directory for relative filenames.
Signed-off-by: Jeff King <peff@peff.net>
---
git-cvsimport.perl | 17 ++++++++++++++---
1 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 1ad20ac..a7d215c 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -579,10 +579,21 @@ sub get_headref ($) {
return $r;
}
+my $user_filename_prepend = '';
+sub munge_user_filename {
+ my $name = shift;
+ return File::Spec->file_name_is_absolute($name) ?
+ $name :
+ $user_filename_prepend . $name;
+}
+
-d $git_tree
or mkdir($git_tree,0777)
or die "Could not create $git_tree: $!";
-chdir($git_tree);
+if ($git_tree ne '.') {
+ $user_filename_prepend = getwd() . '/';
+ chdir($git_tree);
+}
my $last_branch = "";
my $orig_branch = "";
@@ -644,7 +655,7 @@ unless (-d $git_dir) {
-f "$git_dir/cvs-authors" and
read_author_info("$git_dir/cvs-authors");
if ($opt_A) {
- read_author_info($opt_A);
+ read_author_info(munge_user_filename($opt_A));
write_author_info("$git_dir/cvs-authors");
}
@@ -679,7 +690,7 @@ unless ($opt_P) {
$? == 0 or die "git-cvsimport: fatal: cvsps reported error\n";
close $cvspsfh;
} else {
- $cvspsfile = $opt_P;
+ $cvspsfile = munge_user_filename($opt_P);
}
open(CVS, "<$cvspsfile") or die $!;
--
1.6.5.1.121.g1a4d5
^ permalink raw reply related
* Re: [PATCH] git add -e documentation: rephrase note
From: Jeff King @ 2009-10-19 6:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Miklos Vajna, git
In-Reply-To: <7vbpk4aqop.fsf@alter.siamese.dyndns.org>
On Sun, Oct 18, 2009 at 10:38:46PM -0700, Junio C Hamano wrote:
> > -*NOTE*: Obviously, if you change anything else than the first character
> > -on lines beginning with a space or a minus, the patch will no longer
> > -apply.
> > +The intent of this option is to pick and choose lines of the diff to
>
> I'd slightly prefer "patch" over "diff" in this context.
Sure, I have no problem with that.
> > +apply, or even to modify the contents of lines to be staged. There are
> > +three line types in a diff: addition lines (beginning with a plus),
> > +removal lines (beginning with a minus), and context lines (beginning
> > +with a space). In general, it should be safe to:
> > ++
> > +--
> > +* remove addition lines (don't stage the line)
>
> This is more like "don't add the line", isn't it? Also if this "+" line
> has corresponding "-" line (i.e. it is a "rewrite to this" line), removal
> of such a line would mean "instead of rewriting, remove it".
I was trying not to use "add" because we are already talking about
addition and removal in the patch itself, and I thought it made sense to
map those actions to what the user is conceptually doing (staging or not
staging). Of course, I used "remove" which has a similar problem (you
are removing the addition of the line to the index); probably "delete"
would have been better. So I am not sure I agree that using "add" is any
better than "stage", but I don't feel that strongly about it.
But beyond that, yes, you are right that removing a "+" line may have a
different conceptual meaning to the user depending on the surrounding
text. I wonder if such a "check-list" document really makes much sense,
given that using "-e" at all means you need to understand the patch
format and what makes sense (i.e., anybody who understands 'patch' knows
that you can't just delete context lines and expect it to apply).
> Obviously, the above patch is what "diff --cached" would show after such
> an "add -e", but this does _not_ touch anything in the work tree (which is
> correct; add is about moving changed contents to the index and it should
> fundamentally not affect work tree). As a result, "diff" between the
> index and the work tree now would read like this:
>
> diff --git a/t-f b/t-f
> index 17c3f0b..d68dd40 100644
> --- a/t-f
> +++ b/t-f
> @@ -1,3 +1,4 @@
> a
> -e
> +b
> +c
> d
>
> IOW, your request to "add -e" was "I do not want to put the addition of
> 'c' in the index at this point (that is why you removed '+c')" and "I do
> not want to put the addition of 'b' in the index; I want 'e' instead (that
> is why you changed '+b' to '+e')". After "add -e" granted both requests,
> what is left in the work tree can confuse the user. The "not at this
Yeah, again, this comes down to understanding what you are doing:
applying a patch to the index. So it really requires that the user
understand what that means. Maybe instead of trying to enumerate cases
and their effects, we should just say sometihng like "this is a patch to
the index. While you can make arbitrary changes, keep in mind that (1)
your changes must apply to the content in the index, and (2) any new
changes you introduce in the patch will not be reflected in the working
tree".
-Peff
^ permalink raw reply
* Re: Creating something like increasing revision numbers
From: Johannes Sixt @ 2009-10-19 6:21 UTC (permalink / raw)
To: Norbert Preining; +Cc: Daniel Barkalow, git
In-Reply-To: <20091019004447.GC11739@gamma.logic.tuwien.ac.at>
Norbert Preining schrieb:
> On So, 18 Okt 2009, Daniel Barkalow wrote:
>> of the *content* is non-linear, the sequence of values stored in
>> refs/heads/master on your central server is linear, local, and easy to
>> enumerate.
>
> That is exactely what I need.
You can always run 'git rev-list master | wc -l' to get a sequence number.
You can throw in --first-parent if you do not want to count commits that
entered master through a merge commit.
You can configure your reflogs such that they do not expire. Then you
count the entries in the reflog.
-- Hannes
^ permalink raw reply
* Re: [PATCH] Use "--no-" prefix to switch off some of checkout dwimmery
From: Junio C Hamano @ 2009-10-19 6:16 UTC (permalink / raw)
To: Alex Riesen; +Cc: Junio C Hamano, git, Johannes Schindelin, Jay Soffian
In-Reply-To: <81b0412b0910182312h583e74e4v2678eb4375164c34@mail.gmail.com>
Alex Riesen <raa.lkml@gmail.com> writes:
> On Mon, Oct 19, 2009 at 08:07, Alex Riesen <raa.lkml@gmail.com> wrote:
>> On Mon, Oct 19, 2009 at 00:49, Junio C Hamano <gitster@pobox.com> wrote:
>>> Alex Riesen <raa.lkml@gmail.com> writes:
>>>> + OPT_SET_INT(0, "dwim", &dwim_new_local_branch,
>>>> + "Guess local branch from remote reference (default)", 0),
>>>
>>> Humph, how does SET_INT know to set it to 1 with --dwim and set it to 0
>>> with --no-dwim?
>>
>> It seems to do, though (I checked before sending).
>>
>
> Right, just looked at the parse-options: it is defined for all types.
>
> parse-options.c +/get_value
>
> const int unset = flags & OPT_UNSET;
> ...
> case OPTION_SET_INT:
> *(int *)opt->value = unset ? 0 : opt->defval;
> return 0;
>
> Very useful.
Ah, did you mean to change the default value to 1 as well?
^ permalink raw reply
* Re: [PATCH] Use "--no-" prefix to switch off some of checkout dwimmery
From: Alex Riesen @ 2009-10-19 6:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Schindelin, Jay Soffian
In-Reply-To: <81b0412b0910182307n53b4a51cvaa14829ea8b40207@mail.gmail.com>
On Mon, Oct 19, 2009 at 08:07, Alex Riesen <raa.lkml@gmail.com> wrote:
> On Mon, Oct 19, 2009 at 00:49, Junio C Hamano <gitster@pobox.com> wrote:
>> Alex Riesen <raa.lkml@gmail.com> writes:
>>> + OPT_SET_INT(0, "dwim", &dwim_new_local_branch,
>>> + "Guess local branch from remote reference (default)", 0),
>>
>> Humph, how does SET_INT know to set it to 1 with --dwim and set it to 0
>> with --no-dwim?
>
> It seems to do, though (I checked before sending).
>
Right, just looked at the parse-options: it is defined for all types.
parse-options.c +/get_value
const int unset = flags & OPT_UNSET;
...
case OPTION_SET_INT:
*(int *)opt->value = unset ? 0 : opt->defval;
return 0;
Very useful.
^ permalink raw reply
* Re: [PATCH] Use "--no-" prefix to switch off some of checkout dwimmery
From: Alex Riesen @ 2009-10-19 6:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Schindelin, Jay Soffian
In-Reply-To: <7vzl7omi5z.fsf@alter.siamese.dyndns.org>
On Mon, Oct 19, 2009 at 00:49, Junio C Hamano <gitster@pobox.com> wrote:
> Alex Riesen <raa.lkml@gmail.com> writes:
>> + OPT_SET_INT(0, "dwim", &dwim_new_local_branch,
>> + "Guess local branch from remote reference (default)", 0),
>
> Humph, how does SET_INT know to set it to 1 with --dwim and set it to 0
> with --no-dwim?
It seems to do, though (I checked before sending).
^ permalink raw reply
* Re: [PATCH] Document git push -q
From: Junio C Hamano @ 2009-10-19 5:58 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Junio C Hamano, Sebastian Pipping, Jeff King, git
In-Reply-To: <20091018235240.GU6115@genesis.frugalware.org>
Thanks; applied.
^ permalink raw reply
* Re: [PATCH] Makefile: clean block-sha1/ directory instead of mozilla-sha1/
From: Junio C Hamano @ 2009-10-19 5:58 UTC (permalink / raw)
To: Carlos R. Mafra; +Cc: git
In-Reply-To: <20091011133219.GA28070@Pilar.aei.mpg.de>
Thanks, applied.
^ permalink raw reply
* Re: [PATCH 2/3] DWIM "git checkout frotz" to "git checkout -b frotz origin/frotz"
From: Björn Steinbrink @ 2009-10-19 5:58 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: Junio C Hamano, git, Johannes Schindelin, Jay Soffian
In-Reply-To: <20091019052043.6117@nanako3.lavabit.com>
On 2009.10.19 05:20:43 +0900, Nanako Shiraishi wrote:
> A user who always wants tracking can set the config option and use
> the new "git checkout frotz" shortcut, but a user who usually
> doesn't want tracking doesn't have the config option and when he
> wants tracking only for this new branch he can explicitly say "git
> checkout -t origin/frotz", right?
Well, branch.autosetupmerge has three possible values.
- true: Do the upstream setup when starting from remote tracking
branches
- always: Also do the upstream setup when starting from a local branch
head
- false: Don't do any upstream setup
The default is "true", which should catch the "git checkout frotz"
shortcut, as that selects a remote tracking branch as the starting
point. So the user doesn't have to change any config setting to have
that act as if -t was given.
Only if we doesn't want "git checkout frotz" to not do the upstream
setup, he needs to set branch.autosetupmerge to false.
And falling back to "git checkout --track/--no-track origin/frotz" he
can override whatever config setting he has.
Björn
^ permalink raw reply
* Re: [PATCH] git push: remove incomplete options list from help text
From: Junio C Hamano @ 2009-10-19 5:41 UTC (permalink / raw)
To: Jeff King
Cc: Nanako Shiraishi, Junio C Hamano, Miklos Vajna, Sebastian Pipping,
git
In-Reply-To: <20091019041033.GB7170@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Probably few people read it, as it was buried deep in a thread. But
> maybe we should settle on a rule like "short synopsis for usage, long
> synopsis for manpage" or whatever people think is best.
>
> Also, minor nit with your patch: should it be "[<options>]"?
Thanks, I agree with you on both counts.
^ permalink raw reply
* Re: [PATCH] git add -e documentation: rephrase note
From: Junio C Hamano @ 2009-10-19 5:38 UTC (permalink / raw)
To: Jeff King; +Cc: Miklos Vajna, Junio C Hamano, git
In-Reply-To: <20091019050456.GA15706@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Hmph. Here is my attempt. The text is (I hope) more clear, but I am
> still having trouble with the formatting. It looks fine in the HTML
> version, and if I am reading the XML right, the XML is correct. But
> docbook seems to screw up converting the XML to roff, giving this:
>
> · convert removal lines to context lines (don’t stage removal)
> Similarly, your patch will likely not apply if you:
>
> · add context or removal lines
>
> Am I missing something, or is it really a docbook bug? Does it render
> better for anybody else?
Not for me, and I have seen manpages lacking a blank line like the above
where I expect one in other places, so it is not very surprising.
> -- >8 --
> Subject: [PATCH] docs: give more hints about how "add -e" works
>
> The original text was not very descriptive about what you
> can and can't do; let's try to enumerate all cases.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> Documentation/git-add.txt | 22 +++++++++++++++++++---
> 1 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
> index 45ebf87..b0a8420 100644
> --- a/Documentation/git-add.txt
> +++ b/Documentation/git-add.txt
> @@ -86,9 +86,25 @@ OPTIONS
> edit it. After the editor was closed, adjust the hunk headers
> and apply the patch to the index.
> +
> -*NOTE*: Obviously, if you change anything else than the first character
> -on lines beginning with a space or a minus, the patch will no longer
> -apply.
> +The intent of this option is to pick and choose lines of the diff to
I'd slightly prefer "patch" over "diff" in this context.
> +apply, or even to modify the contents of lines to be staged. There are
> +three line types in a diff: addition lines (beginning with a plus),
> +removal lines (beginning with a minus), and context lines (beginning
> +with a space). In general, it should be safe to:
> ++
> +--
> +* remove addition lines (don't stage the line)
This is more like "don't add the line", isn't it? Also if this "+" line
has corresponding "-" line (i.e. it is a "rewrite to this" line), removal
of such a line would mean "instead of rewriting, remove it".
> +* modify the content of any addition lines (stage modified contents)
Would be "add differently".
> +* add new addition lines (stage the new line)
"Add more than what you have in the work tree"
While kibitzing like the above, I noticed that there is another thing that
may deserve warning even more. Suppose you added lines and the patch
between HEAD and the work tree looks like this:
diff --git a/t-f b/t-f
index e69de29..d68dd40 100644
--- a/t-f
+++ b/t-f
@@ -0,0 +1,4 @@
+a
+b
+c
+d
And you do "add -e" and edit the patch to:
diff --git a/t-f b/t-f
index e69de29..17c3f0b 100644
--- a/t-f
+++ b/t-f
@@ -0,0 +1,3 @@
+a
+e
+d
Obviously, the above patch is what "diff --cached" would show after such
an "add -e", but this does _not_ touch anything in the work tree (which is
correct; add is about moving changed contents to the index and it should
fundamentally not affect work tree). As a result, "diff" between the
index and the work tree now would read like this:
diff --git a/t-f b/t-f
index 17c3f0b..d68dd40 100644
--- a/t-f
+++ b/t-f
@@ -1,3 +1,4 @@
a
-e
+b
+c
d
IOW, your request to "add -e" was "I do not want to put the addition of
'c' in the index at this point (that is why you removed '+c')" and "I do
not want to put the addition of 'b' in the index; I want 'e' instead (that
is why you changed '+b' to '+e')". After "add -e" granted both requests,
what is left in the work tree can confuse the user. The "not at this
point" part of the first request is clearly visible that the leftover diff
has an addition of '+c'. But the user may expect that the second request,
"I don't want 'b', I want 'e'", would carry over to the work tree and not
see the apparent reversion of the wish (i.e. you changed it to add 'e'
instead of 'b' in "add -e" session, but it is reverted and "commit -a"
would record the version with 'b' instead).
^ permalink raw reply
* Re: git push should automatically push to remote tracking branch
From: Jeff King @ 2009-10-19 5:06 UTC (permalink / raw)
To: Mohit Aron; +Cc: Tomas Carnecky, git
In-Reply-To: <ee22b09e0910182050k55efac83v6799285d992fcbb0@mail.gmail.com>
On Sun, Oct 18, 2009 at 08:50:57PM -0700, Mohit Aron wrote:
> Thanks - I suppose this is a new config option. I'm running Git
> version 1.6.0.4 (the latest with Ubuntu Jaunty) and that doesn't seem
> to have this option.
Yes, it was added in v1.6.3.
-Peff
^ permalink raw reply
* Re: [PATCH] git add -e documentation: rephrase note
From: Jeff King @ 2009-10-19 5:04 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Junio C Hamano, git
In-Reply-To: <20091019043418.GD7170@coredump.intra.peff.net>
On Mon, Oct 19, 2009 at 12:34:18AM -0400, Jeff King wrote:
> I still have a few comments, though.
> [...]
> (the spacing of which will hopefully not be destroyed in transit). In
> other words, the "+" list continuation makes the "it is a bad idea" text
> part of the "good idea" list, instead of continuing the definition of
> the "-e" option. I think we can fix it with an open block marker. I'll
> see what I can do.
Hmph. Here is my attempt. The text is (I hope) more clear, but I am
still having trouble with the formatting. It looks fine in the HTML
version, and if I am reading the XML right, the XML is correct. But
docbook seems to screw up converting the XML to roff, giving this:
· convert removal lines to context lines (don’t stage removal)
Similarly, your patch will likely not apply if you:
· add context or removal lines
Am I missing something, or is it really a docbook bug? Does it render
better for anybody else?
-- >8 --
Subject: [PATCH] docs: give more hints about how "add -e" works
The original text was not very descriptive about what you
can and can't do; let's try to enumerate all cases.
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/git-add.txt | 22 +++++++++++++++++++---
1 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 45ebf87..b0a8420 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -86,9 +86,25 @@ OPTIONS
edit it. After the editor was closed, adjust the hunk headers
and apply the patch to the index.
+
-*NOTE*: Obviously, if you change anything else than the first character
-on lines beginning with a space or a minus, the patch will no longer
-apply.
+The intent of this option is to pick and choose lines of the diff to
+apply, or even to modify the contents of lines to be staged. There are
+three line types in a diff: addition lines (beginning with a plus),
+removal lines (beginning with a minus), and context lines (beginning
+with a space). In general, it should be safe to:
++
+--
+* remove addition lines (don't stage the line)
+* modify the content of any addition lines (stage modified contents)
+* add new addition lines (stage the new line)
+* convert context lines to removal lines (stage removal of line)
+* convert removal lines to context lines (don't stage removal)
+--
++
+Similarly, your patch will likely not apply if you:
++
+* add context or removal lines
+* delete removal or context lines
+* modify the contents of context or removal lines
-u::
--update::
--
1.6.5.1.123.gcaaf
^ permalink raw reply related
* Re: [PATCH] git add -e documentation: rephrase note
From: Jeff King @ 2009-10-19 4:34 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Junio C Hamano, git
In-Reply-To: <20091019000900.GV6115@genesis.frugalware.org>
On Mon, Oct 19, 2009 at 02:09:00AM +0200, Miklos Vajna wrote:
> Add a quick overview about what is OK and what is not to cover all
> cases.
>
> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> Signed-off-by: Jeff King <peff@peff.net>
Please be careful with sign-offs. While your patch text is an adaptation
of what I said, and while it is true that I personally would never say
anything on the list that was not free for use in git, I did not in fact
do such a sign-off, which is what that line is supposed to be
documenting.
In this case, I think you probably just wanted to say "text from Jeff
King" or even "From: Jeff King <peff@peff.net>".
That being said,
Signed-off-by: Jeff King <peff@peff.net>
:)
I still have a few comments, though.
> +The intent of this option is to pick and choose lines of the diff to
> +apply, or even to munge the lines. So it is safe to:
> ++
> +* remove lines with a "+" (don't stage the addition)
> +* munge any lines with a "+" (stage modified contents)
Do we want to use "munge" here? Maybe something a little more specific
like "modify the content of" is more appropriate for documentation.
> +* add lines with a "+" (stage an addition)
> +* convert lines with a " " to "-" (stage removal)
> +* convert lines with a "-" to " " (don't stage removal)
Is " " going to render in an obvious-to-read way? I checked the html
version and the manpage version in my terminal, and they looked OK, but
I would not be surprised if somebody has a font setup that makes it a
little hard to read.
> ++
> +It is a bad idea to:
> ++
Annoyingly, this renders for me as:
· convert lines with a "-" to " " (don’t stage removal)
It is a bad idea to:
· delete "-" lines
(the spacing of which will hopefully not be destroyed in transit). In
other words, the "+" list continuation makes the "it is a bad idea" text
part of the "good idea" list, instead of continuing the definition of
the "-e" option. I think we can fix it with an open block marker. I'll
see what I can do.
-Peff
^ permalink raw reply
* Re: [PATCH] git push: say that --tag can't be used with --all or --mirror in help text
From: Jeff King @ 2009-10-19 4:14 UTC (permalink / raw)
To: Nanako Shiraishi
Cc: Junio C Hamano, Miklos Vajna, Sebastian Pipping, git,
Bjorn Gustavsson
In-Reply-To: <20091019125701.6117@nanako3.lavabit.com>
On Mon, Oct 19, 2009 at 12:57:01PM +0900, Nanako Shiraishi wrote:
> - OPT_BOOLEAN( 0 , "tags", &tags, "push tags"),
> + OPT_BOOLEAN( 0 , "tags", &tags, "push tags (can't be used with --all nor --mirror"),
Grammar nit: I believe it should be "or" and not "nor".
There is an implicit "either", as in "can't be used with either --all or
--mirror". Saying "can't be used with neither --all nor --mirror" would
be a double-negative. The alternative correct single-negation would be
"can be used with neither --all nor --mirror".
-Peff
^ permalink raw reply
* Re: [PATCH] git push: remove incomplete options list from help text
From: Jeff King @ 2009-10-19 4:10 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: Junio C Hamano, Miklos Vajna, Sebastian Pipping, git
In-Reply-To: <20091019115412.6117@nanako3.lavabit.com>
On Mon, Oct 19, 2009 at 11:54:12AM +0900, Nanako Shiraishi wrote:
> static const char * const push_usage[] = {
> - "git push [--all | --mirror] [-n | --dry-run] [--porcelain] [--tags] [--receive-pack=<git-receive-pack>] [--repo=<repository>] [-f | --force] [-v] [<repository> <refspec>...]",
> + "git push <options> [<repository> <refspec>...]",
This is a big improvement, IMO. We should probably standardize on when
to show options, and when to simply say <options>, and make sure every
program does the right thing. I am in favor of a short synopsis followed
by a list (as you do here) for both usage and for manpages. However, I
raised the question a few weeks ago and the response was slightly
negative:
http://thread.gmane.org/gmane.comp.version-control.git/129399/focus=129424
Probably few people read it, as it was buried deep in a thread. But
maybe we should settle on a rule like "short synopsis for usage, long
synopsis for manpage" or whatever people think is best.
Also, minor nit with your patch: should it be "[<options>]"?
-Peff
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox