* Re: [PATCH] Add Documentation/CodingStyle
From: Johannes Schindelin @ 2007-11-07 14:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ralf Wildenhues, git
In-Reply-To: <7v640ega5q.fsf@gitster.siamese.dyndns.org>
Hi,
On Tue, 6 Nov 2007, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
> > new file mode 100644
> > index 0000000..622b80b
> > --- /dev/null
> > +++ b/Documentation/CodingStyle
> > @@ -0,0 +1,87 @@
> > +As a popular project, we also have some guidelines to keep to the
> > +code. For git in general, two rough rules are:
> > +
> > + - Most importantly, we never say "It's in POSIX; we'll happily
> > + screw your system that does not conform." We live in the
> > + real world.
> > +
> > + - However, we often say "Let's stay away from that construct,
> > + it's not even in POSIX".
> > +
>
> I am not sure if we want to have CodingStyle document, but the
> above are not CodingStyle issues.
Would you like to call it CodingConventions?
> If we are to write this down, I'd like to have the more
> important third rule, which is:
>
> - In spite of the above two rules, we sometimes say "Although
> this is not in POSIX, it (is so convenient | makes the code
> much more readable | has other good characteristics) and
> practically all the platforms we care about support it, so
> let's use it". Again, we live in the real world, and it is
> sometimes a judgement call, decided based more on real world
> constraints people face than what the paper standard says.
Okay, will add.
> > +For C programs:
> > +
> > + - Use tabs to increment, and interpret tabs as taking up to 8 spaces
>
> What's the character for decrement? DEL? ;-)
Hehe. As you undoubtedly guessed, I meant "indent"...
> > + Double negation is often harder to understand than no negation at
> > + all.
> > +
> > + Some clever tricks, like using the !! operator with arithmetic
> > + constructs, can be extremely confusing to others. Avoid them,
> > + unless there is a compelling reason to use them.
>
> I actually think (!!var) idiom is already established in our
> codebase.
Yep, but when there are easier variants, AFAICT they were preferred.
> > + - Use the API. No, really. We have a strbuf (variable length string),
> > + several arrays with the ALLOC_GROW() macro, a path_list for sorted
> > + string lists, a hash map (mapping struct objects) named
> > + "struct decorate", amongst other things.
>
> - When you come up with an API, document it.
Okay.
> > + - if you are planning a new command, consider writing it in shell or
> > + perl first, so that changes in semantics can be easily changed and
> > + discussed. Many git commands started out like that, and a few are
> > + still scripts.
>
> No Python allowed?
Well, maybe we should allow that again. Although I hoped we are over that
now, as it would complicate the msysGit efforts tremendously.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Make git-clean a builtin
From: Shawn Bohrer @ 2007-11-07 14:54 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: gitster, git
In-Reply-To: <Pine.LNX.4.64.0711071110040.4362@racer.site>
On Wed, Nov 07, 2007 at 11:10:45AM +0000, Johannes Schindelin wrote:
>
> you still have quite a number of instances where you wrap just one line
> into curly brackets:
>
> if (bla) {
> [just one line]
> }
Crap. OK I count one instance unless you count:
if (foo) {
one_line();
} else if (bar) {
one_line();
two_lines();
} else {
something_else();
}
Now I suppose I can get rid of the curly braces here as well but I
personally find that strange and ugly. So is there an official guideline
on if else statements?
Of course I'll fix the other one I missed and send a new patch.
^ permalink raw reply
* Re: [PATCH 0/5] some shell portability fixes
From: Johannes Schindelin @ 2007-11-07 14:47 UTC (permalink / raw)
To: Mike Ralphson; +Cc: Ralf Wildenhues, Mike Hommey, Junio C Hamano, git
In-Reply-To: <e2b179460711070617h7e9af64egcde5122808a4d924@mail.gmail.com>
Hi,
On Wed, 7 Nov 2007, Mike Ralphson wrote:
> Junio C Hamano wrote on Tue, Nov 06, 2007 at 09:46:35PM CET:
> > [2/5] Gaah, AIX sed X-<. I am not opposed to this patch but
> > would want to get Yays from people with non GNU sed. Is
> > busybox sed good enough to grok our scripts these days?
> > Please ask help and collect Acks at least from folks on
> > Solaris, MacOS, FBSD, and OBSD.
>
> On Nov 6, 2007 11:25 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > As Junio commented in the part you did not quote, there are better shells
> > in Solaris. Use those.
>
> Equally GNU sed is available as a drop-in rpm for AIX. I wonder if it
> would be worth adding
> Makefile support for a PATH prefix for the git scripts, so they could
> prepend (in this case)
> something like /opt/freeware/bin or /usr/linux/bin ?
>
> In our AIX environment many GNU tools are installed but I can't
> guarantee they come first
> in the paths of the git users.
>
> I'm willing to work up a patch if there's any interest.
Would that be a task for configure? Because I am not sure if the GNU
tools are installed in the same place on all AIX boxen...
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Make git-clean a builtin
From: Matthieu Moy @ 2007-11-07 14:45 UTC (permalink / raw)
To: Bill Lear; +Cc: Johannes Schindelin, Shawn Bohrer, gitster, git
In-Reply-To: <18225.48553.44088.269677@lisa.zopyra.com>
Bill Lear <rael@zopyra.com> writes:
> I've always found this a thoughtful practice. It helps ensure nobody writes:
>
> if (bla)
> just_one_line();
> /* perhaps a comment, other stuff ... */
> just_another_line();
It also simplify patches for cases like
if (bla) {
just_one_line();
+ another_added_line();
}
instead of
- if (bla)
+ if (bla) {
just_one_line();
+ another_added_line();
+ }
But it seems people here prefer not putting the braces in this case.
--
Matthieu
^ permalink raw reply
* Re: [PATCH 0/5] some shell portability fixes
From: Mike Ralphson @ 2007-11-07 14:17 UTC (permalink / raw)
To: Johannes Schindelin, Ralf Wildenhues; +Cc: Mike Hommey, Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0711062324590.4362@racer.site>
Junio C Hamano wrote on Tue, Nov 06, 2007 at 09:46:35PM CET:
> [2/5] Gaah, AIX sed X-<. I am not opposed to this patch but
> would want to get Yays from people with non GNU sed. Is
> busybox sed good enough to grok our scripts these days?
> Please ask help and collect Acks at least from folks on
> Solaris, MacOS, FBSD, and OBSD.
On Nov 6, 2007 11:25 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> As Junio commented in the part you did not quote, there are better shells
> in Solaris. Use those.
Equally GNU sed is available as a drop-in rpm for AIX. I wonder if it
would be worth adding
Makefile support for a PATH prefix for the git scripts, so they could
prepend (in this case)
something like /opt/freeware/bin or /usr/linux/bin ?
In our AIX environment many GNU tools are installed but I can't
guarantee they come first
in the paths of the git users.
I'm willing to work up a patch if there's any interest.
Mike
^ permalink raw reply
* Re: [PATCH] Make git-clean a builtin
From: Johannes Schindelin @ 2007-11-07 14:17 UTC (permalink / raw)
To: Bill Lear; +Cc: Shawn Bohrer, gitster, git
In-Reply-To: <18225.48553.44088.269677@lisa.zopyra.com>
Hi,
On Wed, 7 Nov 2007, Bill Lear wrote:
> On Wednesday, November 7, 2007 at 11:10:45 (+0000) Johannes Schindelin writes:
>
> > you still have quite a number of instances where you wrap just one
> > line into curly brackets:
> >
> > if (bla) {
> > [just one line]
> > }
>
> I've always found this a thoughtful practice. It helps ensure nobody
> writes:
>
> if (bla)
> just_one_line();
> /* perhaps a comment, other stuff ... */
> just_another_line();
But if there is only one line and you fail to add curly brackets when
adding a second line, well, uhm, then I cannot help you with anything.
BTW I was talking about _one_ line, not a line and another one with a
comment.
> It also is nice for others who come along and extend the branch from
> just one line to multiple ones, as the brackets are already in place.
The fact is: these lines will stay single lines most likely for eternity.
> Why do you find it objectionable?
It distracts. It's ugly. It's unnecessary.
Ciao,
Dscho
^ permalink raw reply
* stgit: cleaning up after using git branch delete commands
From: Jon Smirl @ 2007-11-07 14:06 UTC (permalink / raw)
To: Git Mailing List
I've used git commands to delete several branches that had stgit
active on it. Doing that has left a bunch of clutter in the .git
directory. Is there a stgit command to remove all the clutter from
branches that no longer exist? I'd like to use the branch names again
but the clutter is interfering.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [PATCH] Make git-clean a builtin
From: Bill Lear @ 2007-11-07 13:29 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Shawn Bohrer, gitster, git
In-Reply-To: <Pine.LNX.4.64.0711071110040.4362@racer.site>
On Wednesday, November 7, 2007 at 11:10:45 (+0000) Johannes Schindelin writes:
>Hi,
>
>you still have quite a number of instances where you wrap just one line
>into curly brackets:
>
> if (bla) {
> [just one line]
> }
I've always found this a thoughtful practice. It helps ensure nobody writes:
if (bla)
just_one_line();
/* perhaps a comment, other stuff ... */
just_another_line();
which I've seen happen countless times. It also is nice for others who
come along and extend the branch from just one line to multiple ones,
as the brackets are already in place.
Why do you find it objectionable?
Bill
^ permalink raw reply
* Re: [PATCH] Make git-clean a builtin
From: Johannes Schindelin @ 2007-11-07 11:10 UTC (permalink / raw)
To: Shawn Bohrer; +Cc: gitster, git
In-Reply-To: <11944127311587-git-send-email-shawn.bohrer@gmail.com>
Hi,
you still have quite a number of instances where you wrap just one line
into curly brackets:
if (bla) {
[just one line]
}
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] git-revert is one of the most misunderstood command in git, help users out.
From: Johannes Schindelin @ 2007-11-07 11:08 UTC (permalink / raw)
To: Mike Hommey
Cc: Robin Rosenberg, Junio C Hamano, Steven Grimm, Pierre Habouzit,
git
In-Reply-To: <20071107081608.GA19066@glandium.org>
Hi,
On Wed, 7 Nov 2007, Mike Hommey wrote:
> On Tue, Nov 06, 2007 at 10:25:48PM +0000, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> > On Tue, 6 Nov 2007, Robin Rosenberg wrote:
> >
> > > tisdag 06 november 2007 skrev Mike Hommey:
> > > > Maybe the documentation could emphasise on how to undo things when
> > > > the user makes mistakes. Sometimes, saving your repo can be as
> > > > simple as git reset --hard HEAD@{1}. This is not, unfortunately, a
> > > > works-for-all-cases command.
> > >
> > > Yea, git-undo(7).
> >
> > In related news, I know a few users who need an un-rm-rf. Anyone?
>
> The fact is you can do harm to your repo with things you wouldn't expect
> to break things, except maybe you gave bad arguments or so. It's quite
> easy to fuck up with git-rebase, or to merge the wrong commits, etc.
I don't see how these commands are dangerous. Usually you just look into
the reflog, pick the one commit you started with, and reset --hard.
The _only_ commands I find dangerous are "git stash clear" and "git reflog
--expire=0". Funnily, people want to do that all the time.
Like recently, on the IRC channel, where somebody lost patches "during a
rebase", by "rm -rf .dotest".
There will be a point where nobody can help. But before that, reflogs are
your friend. But you must not do "reset --hard HEAD@{1}" blindly. You
have to look first what the reflogs are.
Ciao,
Dscho
^ permalink raw reply
* Re: [ANNOUNCE] cgit v0.7
From: Lars Hjemli @ 2007-11-07 10:52 UTC (permalink / raw)
To: Patrick Aljord; +Cc: git list
In-Reply-To: <6b6419750711061039l26290561wd2abe07035a8679c@mail.gmail.com>
On Nov 6, 2007 7:39 PM, Patrick Aljord <patcito@gmail.com> wrote:
> Looks great, thanks for the new release.
Thanks!
> It would be great if when a commit message contains something such as
> #1234 it would automatically link to the bug tracker page of that bug
> (like Trac does) by preconfiguring the bugtracker URL.
Yeah, it shouldn't be to difficult either. We could just add a couple
of repo-options to cgitrc, maybe something like this:
repo.bugtracker-regex=(#[0-9]+)
repo.bugtracker-url=http://bugs.freedesktop.org/show_bug.cgi?id=%1
and then use regexec() combined with the interpolate() function in git
to perform the substitution. There could also be similar global
options, i.e.:
bugtracker-regex=(#[0-9]+)
bugtracker-url=http://bugs.freedesktop.org/show_bug.cgi?id=%1
which would be used if the per-repo options weren't specified.
I'll put it on my todo-list (and patches are always appreciated).
--
larsh
^ permalink raw reply
* [PATCH] hooks--update: allow deleting a tag through git push <remote> :<tag>
From: Gerrit Pape @ 2007-11-07 10:34 UTC (permalink / raw)
To: git, Junio C Hamano
The update hook interpretes deleting a tag, no matter if annotated or not,
through git push <remote> :<tag> as unannotated tag, and declines it by
default:
$ git push origin :atag
deleting 'refs/tags/atag'
*** The un-annotated tag, atag, is not allowed in this repository
*** Use 'git tag [ -a | -s ]' for tags you want to propagate.
ng refs/tags/atag hook declined
error: hooks/update exited with error code 1
error: hook declined to update refs/tags/atag
error: failed to push to 'monolith:/git/qm/test-repo'
With this patch deleting a tag is allowed unconditionally, just as
deleting a branch.
Signed-off-by: Gerrit Pape <pape@smarden.org>
---
templates/hooks--update | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/templates/hooks--update b/templates/hooks--update
index 65e8c32..a109b1b 100644
--- a/templates/hooks--update
+++ b/templates/hooks--update
@@ -43,7 +43,7 @@ fi
# --- Check types
# if $newrev is 0000...0000, it's a commit to delete a branch
if [ "$newrev" = "0000000000000000000000000000000000000000" ]; then
- newrev_type=commit
+ newrev_type=delete
else
newrev_type=$(git-cat-file -t $newrev)
fi
@@ -58,13 +58,16 @@ case "$refname","$newrev_type" in
exit 1
fi
;;
+ refs/tags/*,delete)
+ # delete tag
+ ;;
refs/tags/*,tag)
# annotated tag
;;
- refs/heads/*,commit)
+ refs/heads/*,commit | refs/heads/*,delete)
# branch
;;
- refs/remotes/*,commit)
+ refs/remotes/*,commit | refs/remotes/*,delete)
# tracking branch
;;
*)
--
1.5.3.5
^ permalink raw reply related
* [PATCH] gitk: Swap positions of 'next' and 'prev' buttons in the 'Find' section.
From: Johannes Sixt @ 2007-11-07 10:22 UTC (permalink / raw)
To: Paul Mackerras; +Cc: Git Mailing List
The button order 'prev' left of 'next' feels more natural than the other
way round, in particular, compared to the order of the forward and backward
arrows that are in the line above.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
gitk | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/gitk b/gitk
index 01a8541..727f4a0 100755
--- a/gitk
+++ b/gitk
@@ -767,7 +767,7 @@ proc makewindow {} {
button .tf.lbar.fnext -text "next" -command {dofind 1 1} -font uifont
button .tf.lbar.fprev -text "prev" -command {dofind -1 1} -font uifont
label .tf.lbar.flab2 -text " commit " -font uifont
- pack .tf.lbar.flabel .tf.lbar.fnext .tf.lbar.fprev .tf.lbar.flab2 \
+ pack .tf.lbar.flabel .tf.lbar.fprev .tf.lbar.fnext .tf.lbar.flab2 \
-side left -fill y
set gdttype "containing:"
set gm [tk_optionMenu .tf.lbar.gdttype gdttype \
--
1.5.3.5.1298.g228d6-dirty
^ permalink raw reply related
* [PATCH DIFF-CLEANUP 1/2] Make the diff_options bitfields be an unsigned with explicit masks.
From: Pierre Habouzit @ 2007-11-07 10:20 UTC (permalink / raw)
To: gitster; +Cc: git, Pierre Habouzit
In-Reply-To: <1194430832-6224-6-git-send-email-madcoder@debian.org>
reverse_diff was a bit-value in disguise, it's merged in the flags now.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
builtin-blame.c | 10 ++--
builtin-diff-files.c | 4 +-
builtin-diff-index.c | 4 +-
builtin-diff-tree.c | 9 ++--
builtin-diff.c | 12 +++---
builtin-log.c | 24 ++++------
combine-diff.c | 10 ++--
diff-lib.c | 23 +++++-----
diff.c | 123 ++++++++++++++++++++++++-------------------------
diff.h | 40 ++++++++++-------
log-tree.c | 6 +--
merge-recursive.c | 2 +-
patch-ids.c | 2 +-
revision.c | 22 +++++-----
tree-diff.c | 14 +++---
15 files changed, 155 insertions(+), 150 deletions(-)
diff --git a/builtin-blame.c b/builtin-blame.c
index 55a3c0b..eaf9c15 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -335,7 +335,7 @@ static struct origin *find_origin(struct scoreboard *sb,
* same and diff-tree is fairly efficient about this.
*/
diff_setup(&diff_opts);
- diff_opts.recursive = 1;
+ DIFF_OPT_SET(&diff_opts, RECURSIVE);
diff_opts.detect_rename = 0;
diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
paths[0] = origin->path;
@@ -409,7 +409,7 @@ static struct origin *find_rename(struct scoreboard *sb,
const char *paths[2];
diff_setup(&diff_opts);
- diff_opts.recursive = 1;
+ DIFF_OPT_SET(&diff_opts, RECURSIVE);
diff_opts.detect_rename = DIFF_DETECT_RENAME;
diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
diff_opts.single_follow = origin->path;
@@ -1075,7 +1075,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
return 1; /* nothing remains for this target */
diff_setup(&diff_opts);
- diff_opts.recursive = 1;
+ DIFF_OPT_SET(&diff_opts, RECURSIVE);
diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
paths[0] = NULL;
@@ -1093,7 +1093,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
if ((opt & PICKAXE_BLAME_COPY_HARDEST)
|| ((opt & PICKAXE_BLAME_COPY_HARDER)
&& (!porigin || strcmp(target->path, porigin->path))))
- diff_opts.find_copies_harder = 1;
+ DIFF_OPT_SET(&diff_opts, FIND_COPIES_HARDER);
if (is_null_sha1(target->commit->object.sha1))
do_diff_cache(parent->tree->object.sha1, &diff_opts);
@@ -1102,7 +1102,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
target->commit->tree->object.sha1,
"", &diff_opts);
- if (!diff_opts.find_copies_harder)
+ if (!DIFF_OPT_TST(&diff_opts, FIND_COPIES_HARDER))
diffcore_std(&diff_opts);
retval = 0;
diff --git a/builtin-diff-files.c b/builtin-diff-files.c
index 6cb30c8..046b7e3 100644
--- a/builtin-diff-files.c
+++ b/builtin-diff-files.c
@@ -31,5 +31,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
if (!rev.diffopt.output_format)
rev.diffopt.output_format = DIFF_FORMAT_RAW;
result = run_diff_files_cmd(&rev, argc, argv);
- return rev.diffopt.exit_with_status ? rev.diffopt.has_changes: result;
+ if (DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))
+ return DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0;
+ return result;
}
diff --git a/builtin-diff-index.c b/builtin-diff-index.c
index 81e7167..556c506 100644
--- a/builtin-diff-index.c
+++ b/builtin-diff-index.c
@@ -44,5 +44,7 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
return -1;
}
result = run_diff_index(&rev, cached);
- return rev.diffopt.exit_with_status ? rev.diffopt.has_changes: result;
+ if (DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))
+ return DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0;
+ return result;
}
diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c
index 0b591c8..e71841a 100644
--- a/builtin-diff-tree.c
+++ b/builtin-diff-tree.c
@@ -118,12 +118,12 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
}
if (!read_stdin)
- return opt->diffopt.exit_with_status ?
- opt->diffopt.has_changes: 0;
+ return DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS)
+ && DIFF_OPT_TST(&opt->diffopt, HAS_CHANGES);
if (opt->diffopt.detect_rename)
opt->diffopt.setup |= (DIFF_SETUP_USE_SIZE_CACHE |
- DIFF_SETUP_USE_CACHE);
+ DIFF_SETUP_USE_CACHE);
while (fgets(line, sizeof(line), stdin)) {
unsigned char sha1[20];
@@ -134,5 +134,6 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
else
diff_tree_stdin(line);
}
- return opt->diffopt.exit_with_status ? opt->diffopt.has_changes: 0;
+ return DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS)
+ && DIFF_OPT_TST(&opt->diffopt, HAS_CHANGES);
}
diff --git a/builtin-diff.c b/builtin-diff.c
index 80392a8..59b9c6e 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -35,7 +35,7 @@ static void stuff_change(struct diff_options *opt,
!hashcmp(old_sha1, new_sha1) && (old_mode == new_mode))
return;
- if (opt->reverse_diff) {
+ if (DIFF_OPT_TST(opt, REVERSE_DIFF)) {
unsigned tmp;
const unsigned char *tmp_u;
const char *tmp_c;
@@ -257,13 +257,13 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
if (diff_setup_done(&rev.diffopt) < 0)
die("diff_setup_done failed");
}
- rev.diffopt.allow_external = 1;
- rev.diffopt.recursive = 1;
+ DIFF_OPT_SET(&rev.diffopt, ALLOW_EXTERNAL);
+ DIFF_OPT_SET(&rev.diffopt, RECURSIVE);
/* If the user asked for our exit code then don't start a
* pager or we would end up reporting its exit code instead.
*/
- if (!rev.diffopt.exit_with_status)
+ if (!DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))
setup_pager();
/* Do we have --cached and not have a pending object, then
@@ -367,8 +367,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
else
result = builtin_diff_combined(&rev, argc, argv,
ent, ents);
- if (rev.diffopt.exit_with_status)
- result = rev.diffopt.has_changes;
+ if (DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))
+ result = DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0;
if (1 < rev.diffopt.skip_stat_unmatch)
refresh_index_quietly();
diff --git a/builtin-log.c b/builtin-log.c
index b6ca04a..72745cd 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -55,13 +55,13 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
rev->abbrev = DEFAULT_ABBREV;
rev->commit_format = CMIT_FMT_DEFAULT;
rev->verbose_header = 1;
- rev->diffopt.recursive = 1;
+ DIFF_OPT_SET(&rev->diffopt, RECURSIVE);
rev->show_root_diff = default_show_root;
rev->subject_prefix = fmt_patch_subject_prefix;
argc = setup_revisions(argc, argv, rev, "HEAD");
if (rev->diffopt.pickaxe || rev->diffopt.filter)
rev->always_show_header = 0;
- if (rev->diffopt.follow_renames) {
+ if (DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES)) {
rev->always_show_header = 0;
if (rev->diffopt.nr_paths != 1)
usage("git logs can only follow renames on one pathname at a time");
@@ -308,11 +308,9 @@ int cmd_show(int argc, const char **argv, const char *prefix)
struct tag *t = (struct tag *)o;
printf("%stag %s%s\n\n",
- diff_get_color(rev.diffopt.color_diff,
- DIFF_COMMIT),
+ diff_get_color_opt(&rev.diffopt, DIFF_COMMIT),
t->tag,
- diff_get_color(rev.diffopt.color_diff,
- DIFF_RESET));
+ diff_get_color_opt(&rev.diffopt, DIFF_RESET));
ret = show_object(o->sha1, 1);
objects[i].item = (struct object *)t->tagged;
i--;
@@ -320,11 +318,9 @@ int cmd_show(int argc, const char **argv, const char *prefix)
}
case OBJ_TREE:
printf("%stree %s%s\n\n",
- diff_get_color(rev.diffopt.color_diff,
- DIFF_COMMIT),
+ diff_get_color_opt(&rev.diffopt, DIFF_COMMIT),
name,
- diff_get_color(rev.diffopt.color_diff,
- DIFF_RESET));
+ diff_get_color_opt(&rev.diffopt, DIFF_RESET));
read_tree_recursive((struct tree *)o, "", 0, 0, NULL,
show_tree_object);
break;
@@ -620,7 +616,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
rev.combine_merges = 0;
rev.ignore_merges = 1;
rev.diffopt.msg_sep = "";
- rev.diffopt.recursive = 1;
+ DIFF_OPT_SET(&rev.diffopt, RECURSIVE);
rev.subject_prefix = fmt_patch_subject_prefix;
rev.extra_headers = extra_headers;
@@ -728,8 +724,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
if (!rev.diffopt.output_format)
rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY | DIFF_FORMAT_PATCH;
- if (!rev.diffopt.text)
- rev.diffopt.binary = 1;
+ if (!DIFF_OPT_TST(&rev.diffopt, TEXT))
+ DIFF_OPT_SET(&rev.diffopt, BINARY);
if (!output_directory && !use_stdout)
output_directory = prefix;
@@ -887,7 +883,7 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
revs.diff = 1;
revs.combine_merges = 0;
revs.ignore_merges = 1;
- revs.diffopt.recursive = 1;
+ DIFF_OPT_SET(&revs.diffopt, RECURSIVE);
if (add_pending_commit(head, &revs, 0))
die("Unknown commit %s", head);
diff --git a/combine-diff.c b/combine-diff.c
index fe5a2a1..3cab04b 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -664,7 +664,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
int mode_differs = 0;
int i, show_hunks;
int working_tree_file = is_null_sha1(elem->sha1);
- int abbrev = opt->full_index ? 40 : DEFAULT_ABBREV;
+ int abbrev = DIFF_OPT_TST(opt, FULL_INDEX) ? 40 : DEFAULT_ABBREV;
mmfile_t result_file;
context = opt->context;
@@ -784,7 +784,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
if (show_hunks || mode_differs || working_tree_file) {
const char *abb;
- int use_color = opt->color_diff;
+ int use_color = DIFF_OPT_TST(opt, COLOR_DIFF);
const char *c_meta = diff_get_color(use_color, DIFF_METAINFO);
const char *c_reset = diff_get_color(use_color, DIFF_RESET);
int added = 0;
@@ -836,7 +836,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
dump_quoted_path("+++ /dev/", "null", c_meta, c_reset);
else
dump_quoted_path("+++ b/", elem->path, c_meta, c_reset);
- dump_sline(sline, cnt, num_parent, opt->color_diff);
+ dump_sline(sline, cnt, num_parent, DIFF_OPT_TST(opt, COLOR_DIFF));
}
free(result);
@@ -929,8 +929,8 @@ void diff_tree_combined(const unsigned char *sha1,
diffopts = *opt;
diffopts.output_format = DIFF_FORMAT_NO_OUTPUT;
- diffopts.recursive = 1;
- diffopts.allow_external = 0;
+ DIFF_OPT_SET(&diffopts, RECURSIVE);
+ DIFF_OPT_CLR(&diffopts, ALLOW_EXTERNAL);
show_log_first = !!rev->loginfo && !rev->no_commit_id;
needsep = 0;
diff --git a/diff-lib.c b/diff-lib.c
index da55713..69b5dc9 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -121,7 +121,7 @@ static int queue_diff(struct diff_options *o,
} else {
struct diff_filespec *d1, *d2;
- if (o->reverse_diff) {
+ if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
unsigned tmp;
const char *tmp_c;
tmp = mode1; mode1 = mode2; mode2 = tmp;
@@ -188,8 +188,7 @@ static int handle_diff_files_args(struct rev_info *revs,
else if (!strcmp(argv[1], "-n") ||
!strcmp(argv[1], "--no-index")) {
revs->max_count = -2;
- revs->diffopt.exit_with_status = 1;
- revs->diffopt.no_index = 1;
+ revs->diffopt.flags |= DIFF_OPT_EXIT_WITH_STATUS | DIFF_OPT_NO_INDEX;
}
else if (!strcmp(argv[1], "-q"))
*silent = 1;
@@ -207,7 +206,7 @@ static int handle_diff_files_args(struct rev_info *revs,
if (!is_in_index(revs->diffopt.paths[0]) ||
!is_in_index(revs->diffopt.paths[1])) {
revs->max_count = -2;
- revs->diffopt.no_index = 1;
+ DIFF_OPT_SET(&revs->diffopt, NO_INDEX);
}
}
@@ -258,7 +257,7 @@ int setup_diff_no_index(struct rev_info *revs,
break;
} else if (i < argc - 3 && !strcmp(argv[i], "--no-index")) {
i = argc - 3;
- revs->diffopt.exit_with_status = 1;
+ DIFF_OPT_SET(&revs->diffopt, EXIT_WITH_STATUS);
break;
}
if (argc != i + 2 || (!is_outside_repo(argv[i + 1], nongit, prefix) &&
@@ -296,7 +295,7 @@ int setup_diff_no_index(struct rev_info *revs,
else
revs->diffopt.paths = argv + argc - 2;
revs->diffopt.nr_paths = 2;
- revs->diffopt.no_index = 1;
+ DIFF_OPT_SET(&revs->diffopt, NO_INDEX);
revs->max_count = -2;
if (diff_setup_done(&revs->diffopt) < 0)
die("diff_setup_done failed");
@@ -310,7 +309,7 @@ int run_diff_files_cmd(struct rev_info *revs, int argc, const char **argv)
if (handle_diff_files_args(revs, argc, argv, &silent_on_removed))
return -1;
- if (revs->diffopt.no_index) {
+ if (DIFF_OPT_TST(&revs->diffopt, NO_INDEX)) {
if (revs->diffopt.nr_paths != 2)
return error("need two files/directories with --no-index");
if (queue_diff(&revs->diffopt, revs->diffopt.paths[0],
@@ -346,7 +345,8 @@ int run_diff_files(struct rev_info *revs, int silent_on_removed)
struct cache_entry *ce = active_cache[i];
int changed;
- if (revs->diffopt.quiet && revs->diffopt.has_changes)
+ if (DIFF_OPT_TST(&revs->diffopt, QUIET) &&
+ DIFF_OPT_TST(&revs->diffopt, HAS_CHANGES))
break;
if (!ce_path_match(ce, revs->prune_data))
@@ -442,7 +442,7 @@ int run_diff_files(struct rev_info *revs, int silent_on_removed)
continue;
}
changed = ce_match_stat(ce, &st, 0);
- if (!changed && !revs->diffopt.find_copies_harder)
+ if (!changed && !DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
continue;
oldmode = ntohl(ce->ce_mode);
newmode = ntohl(ce_mode_from_stat(ce, st.st_mode));
@@ -561,7 +561,7 @@ static int show_modified(struct rev_info *revs,
oldmode = old->ce_mode;
if (mode == oldmode && !hashcmp(sha1, old->sha1) &&
- !revs->diffopt.find_copies_harder)
+ !DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
return 0;
mode = ntohl(mode);
@@ -581,7 +581,8 @@ static int diff_cache(struct rev_info *revs,
struct cache_entry *ce = *ac;
int same = (entries > 1) && ce_same_name(ce, ac[1]);
- if (revs->diffopt.quiet && revs->diffopt.has_changes)
+ if (DIFF_OPT_TST(&revs->diffopt, QUIET) &&
+ DIFF_OPT_TST(&revs->diffopt, HAS_CHANGES))
break;
if (!ce_path_match(ce, pathspec))
diff --git a/diff.c b/diff.c
index 6bb902f..97c9a59 100644
--- a/diff.c
+++ b/diff.c
@@ -827,10 +827,10 @@ static void show_stats(struct diffstat_t* data, struct diff_options *options)
}
/* Find the longest filename and max number of changes */
- reset = diff_get_color(options->color_diff, DIFF_RESET);
- set = diff_get_color(options->color_diff, DIFF_PLAIN);
- add_c = diff_get_color(options->color_diff, DIFF_FILE_NEW);
- del_c = diff_get_color(options->color_diff, DIFF_FILE_OLD);
+ reset = diff_get_color_opt(options, DIFF_RESET);
+ set = diff_get_color_opt(options, DIFF_PLAIN);
+ add_c = diff_get_color_opt(options, DIFF_FILE_NEW);
+ del_c = diff_get_color_opt(options, DIFF_FILE_OLD);
for (i = 0; i < data->nr; i++) {
struct diffstat_file *file = data->files[i];
@@ -1256,8 +1256,8 @@ static void builtin_diff(const char *name_a,
mmfile_t mf1, mf2;
const char *lbl[2];
char *a_one, *b_two;
- const char *set = diff_get_color(o->color_diff, DIFF_METAINFO);
- const char *reset = diff_get_color(o->color_diff, DIFF_RESET);
+ const char *set = diff_get_color_opt(o, DIFF_METAINFO);
+ const char *reset = diff_get_color_opt(o, DIFF_RESET);
a_one = quote_two("a/", name_a + (*name_a == '/'));
b_two = quote_two("b/", name_b + (*name_b == '/'));
@@ -1290,7 +1290,7 @@ static void builtin_diff(const char *name_a,
goto free_ab_and_return;
if (complete_rewrite) {
emit_rewrite_diff(name_a, name_b, one, two,
- o->color_diff);
+ DIFF_OPT_TST(o, COLOR_DIFF));
o->found_changes = 1;
goto free_ab_and_return;
}
@@ -1299,13 +1299,13 @@ static void builtin_diff(const char *name_a,
if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
die("unable to read files to diff");
- if (!o->text &&
+ if (!DIFF_OPT_TST(o, TEXT) &&
(diff_filespec_is_binary(one) || diff_filespec_is_binary(two))) {
/* Quite common confusing case */
if (mf1.size == mf2.size &&
!memcmp(mf1.ptr, mf2.ptr, mf1.size))
goto free_ab_and_return;
- if (o->binary)
+ if (DIFF_OPT_TST(o, BINARY))
emit_binary_diff(&mf1, &mf2);
else
printf("Binary files %s and %s differ\n",
@@ -1328,7 +1328,7 @@ static void builtin_diff(const char *name_a,
memset(&xecfg, 0, sizeof(xecfg));
memset(&ecbdata, 0, sizeof(ecbdata));
ecbdata.label_path = lbl;
- ecbdata.color_diff = o->color_diff;
+ ecbdata.color_diff = DIFF_OPT_TST(o, COLOR_DIFF);
ecbdata.found_changesp = &o->found_changes;
xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;
xecfg.ctxlen = o->context;
@@ -1344,11 +1344,11 @@ static void builtin_diff(const char *name_a,
ecb.outf = xdiff_outf;
ecb.priv = &ecbdata;
ecbdata.xm.consume = fn_out_consume;
- if (o->color_diff_words)
+ if (DIFF_OPT_TST(o, COLOR_DIFF_WORDS))
ecbdata.diff_words =
xcalloc(1, sizeof(struct diff_words_data));
xdl_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
- if (o->color_diff_words)
+ if (DIFF_OPT_TST(o, COLOR_DIFF_WORDS))
free_diff_words_data(&ecbdata);
}
@@ -1422,7 +1422,7 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
data.xm.consume = checkdiff_consume;
data.filename = name_b ? name_b : name_a;
data.lineno = 0;
- data.color_diff = o->color_diff;
+ data.color_diff = DIFF_OPT_TST(o, COLOR_DIFF);
if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
die("unable to read files to diff");
@@ -1866,7 +1866,7 @@ static void run_diff_cmd(const char *pgm,
struct diff_options *o,
int complete_rewrite)
{
- if (!o->allow_external)
+ if (!DIFF_OPT_TST(o, ALLOW_EXTERNAL))
pgm = NULL;
else {
const char *cmd = external_diff_attr(name);
@@ -1964,9 +1964,9 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)
}
if (hashcmp(one->sha1, two->sha1)) {
- int abbrev = o->full_index ? 40 : DEFAULT_ABBREV;
+ int abbrev = DIFF_OPT_TST(o, FULL_INDEX) ? 40 : DEFAULT_ABBREV;
- if (o->binary) {
+ if (DIFF_OPT_TST(o, BINARY)) {
mmfile_t mf;
if ((!fill_mmfile(&mf, one) && diff_filespec_is_binary(one)) ||
(!fill_mmfile(&mf, two) && diff_filespec_is_binary(two)))
@@ -2058,7 +2058,10 @@ void diff_setup(struct diff_options *options)
options->change = diff_change;
options->add_remove = diff_addremove;
- options->color_diff = diff_use_color_default;
+ if (diff_use_color_default)
+ DIFF_OPT_SET(options, COLOR_DIFF);
+ else
+ DIFF_OPT_CLR(options, COLOR_DIFF);
options->detect_rename = diff_detect_rename_default;
}
@@ -2077,7 +2080,7 @@ int diff_setup_done(struct diff_options *options)
if (count > 1)
die("--name-only, --name-status, --check and -s are mutually exclusive");
- if (options->find_copies_harder)
+ if (DIFF_OPT_TST(options, FIND_COPIES_HARDER))
options->detect_rename = DIFF_DETECT_COPY;
if (options->output_format & (DIFF_FORMAT_NAME |
@@ -2101,12 +2104,12 @@ int diff_setup_done(struct diff_options *options)
DIFF_FORMAT_SHORTSTAT |
DIFF_FORMAT_SUMMARY |
DIFF_FORMAT_CHECKDIFF))
- options->recursive = 1;
+ DIFF_OPT_SET(options, RECURSIVE);
/*
* Also pickaxe would not work very well if you do not say recursive
*/
if (options->pickaxe)
- options->recursive = 1;
+ DIFF_OPT_SET(options, RECURSIVE);
if (options->detect_rename && options->rename_limit < 0)
options->rename_limit = diff_rename_limit_default;
@@ -2128,9 +2131,9 @@ int diff_setup_done(struct diff_options *options)
* to have found. It does not make sense not to return with
* exit code in such a case either.
*/
- if (options->quiet) {
+ if (DIFF_OPT_TST(options, QUIET)) {
options->output_format = DIFF_FORMAT_NO_OUTPUT;
- options->exit_with_status = 1;
+ DIFF_OPT_SET(options, EXIT_WITH_STATUS);
}
/*
@@ -2138,7 +2141,7 @@ int diff_setup_done(struct diff_options *options)
* upon the first hit. We need to run diff as usual.
*/
if (options->pickaxe || options->filter)
- options->quiet = 0;
+ DIFF_OPT_CLR(options, QUIET);
return 0;
}
@@ -2201,15 +2204,12 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
options->output_format |= DIFF_FORMAT_PATCH;
else if (!strcmp(arg, "--raw"))
options->output_format |= DIFF_FORMAT_RAW;
- else if (!strcmp(arg, "--patch-with-raw")) {
+ else if (!strcmp(arg, "--patch-with-raw"))
options->output_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_RAW;
- }
- else if (!strcmp(arg, "--numstat")) {
+ else if (!strcmp(arg, "--numstat"))
options->output_format |= DIFF_FORMAT_NUMSTAT;
- }
- else if (!strcmp(arg, "--shortstat")) {
+ else if (!strcmp(arg, "--shortstat"))
options->output_format |= DIFF_FORMAT_SHORTSTAT;
- }
else if (!prefixcmp(arg, "--stat")) {
char *end;
int width = options->stat_width;
@@ -2241,33 +2241,30 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
options->output_format |= DIFF_FORMAT_CHECKDIFF;
else if (!strcmp(arg, "--summary"))
options->output_format |= DIFF_FORMAT_SUMMARY;
- else if (!strcmp(arg, "--patch-with-stat")) {
+ else if (!strcmp(arg, "--patch-with-stat"))
options->output_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_DIFFSTAT;
- }
else if (!strcmp(arg, "-z"))
options->line_termination = 0;
else if (!prefixcmp(arg, "-l"))
options->rename_limit = strtoul(arg+2, NULL, 10);
else if (!strcmp(arg, "--full-index"))
- options->full_index = 1;
+ DIFF_OPT_SET(options, FULL_INDEX);
else if (!strcmp(arg, "--binary")) {
options->output_format |= DIFF_FORMAT_PATCH;
- options->binary = 1;
- }
- else if (!strcmp(arg, "-a") || !strcmp(arg, "--text")) {
- options->text = 1;
+ DIFF_OPT_SET(options, BINARY);
}
+ else if (!strcmp(arg, "-a") || !strcmp(arg, "--text"))
+ DIFF_OPT_SET(options, TEXT);
else if (!strcmp(arg, "--name-only"))
options->output_format |= DIFF_FORMAT_NAME;
else if (!strcmp(arg, "--name-status"))
options->output_format |= DIFF_FORMAT_NAME_STATUS;
else if (!strcmp(arg, "-R"))
- options->reverse_diff = 1;
+ DIFF_OPT_SET(options, REVERSE_DIFF);
else if (!prefixcmp(arg, "-S"))
options->pickaxe = arg + 2;
- else if (!strcmp(arg, "-s")) {
+ else if (!strcmp(arg, "-s"))
options->output_format |= DIFF_FORMAT_NO_OUTPUT;
- }
else if (!prefixcmp(arg, "-O"))
options->orderfile = arg + 2;
else if (!prefixcmp(arg, "--diff-filter="))
@@ -2277,28 +2274,25 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
else if (!strcmp(arg, "--pickaxe-regex"))
options->pickaxe_opts = DIFF_PICKAXE_REGEX;
else if (!prefixcmp(arg, "-B")) {
- if ((options->break_opt =
- diff_scoreopt_parse(arg)) == -1)
+ if ((options->break_opt = diff_scoreopt_parse(arg)) == -1)
return -1;
}
else if (!prefixcmp(arg, "-M")) {
- if ((options->rename_score =
- diff_scoreopt_parse(arg)) == -1)
+ if ((options->rename_score = diff_scoreopt_parse(arg)) == -1)
return -1;
options->detect_rename = DIFF_DETECT_RENAME;
}
else if (!prefixcmp(arg, "-C")) {
if (options->detect_rename == DIFF_DETECT_COPY)
- options->find_copies_harder = 1;
- if ((options->rename_score =
- diff_scoreopt_parse(arg)) == -1)
+ DIFF_OPT_SET(options, FIND_COPIES_HARDER);
+ if ((options->rename_score = diff_scoreopt_parse(arg)) == -1)
return -1;
options->detect_rename = DIFF_DETECT_COPY;
}
else if (!strcmp(arg, "--find-copies-harder"))
- options->find_copies_harder = 1;
+ DIFF_OPT_SET(options, FIND_COPIES_HARDER);
else if (!strcmp(arg, "--follow"))
- options->follow_renames = 1;
+ DIFF_OPT_SET(options, FOLLOW_RENAMES);
else if (!strcmp(arg, "--abbrev"))
options->abbrev = DEFAULT_ABBREV;
else if (!prefixcmp(arg, "--abbrev=")) {
@@ -2309,9 +2303,9 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
options->abbrev = 40;
}
else if (!strcmp(arg, "--color"))
- options->color_diff = 1;
+ DIFF_OPT_SET(options, COLOR_DIFF);
else if (!strcmp(arg, "--no-color"))
- options->color_diff = 0;
+ DIFF_OPT_CLR(options, COLOR_DIFF);
else if (!strcmp(arg, "-w") || !strcmp(arg, "--ignore-all-space"))
options->xdl_opts |= XDF_IGNORE_WHITESPACE;
else if (!strcmp(arg, "-b") || !strcmp(arg, "--ignore-space-change"))
@@ -2319,17 +2313,17 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
else if (!strcmp(arg, "--ignore-space-at-eol"))
options->xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;
else if (!strcmp(arg, "--color-words"))
- options->color_diff = options->color_diff_words = 1;
+ options->flags |= DIFF_OPT_COLOR_DIFF | DIFF_OPT_COLOR_DIFF_WORDS;
else if (!strcmp(arg, "--no-renames"))
options->detect_rename = 0;
else if (!strcmp(arg, "--exit-code"))
- options->exit_with_status = 1;
+ DIFF_OPT_SET(options, EXIT_WITH_STATUS);
else if (!strcmp(arg, "--quiet"))
- options->quiet = 1;
+ DIFF_OPT_SET(options, QUIET);
else if (!strcmp(arg, "--ext-diff"))
- options->allow_external = 1;
+ DIFF_OPT_SET(options, ALLOW_EXTERNAL);
else if (!strcmp(arg, "--no-ext-diff"))
- options->allow_external = 0;
+ DIFF_OPT_CLR(options, ALLOW_EXTERNAL);
else
return 0;
return 1;
@@ -3084,7 +3078,7 @@ static void diffcore_skip_stat_unmatch(struct diff_options *diffopt)
* to determine how many paths were dirty only
* due to stat info mismatch.
*/
- if (!diffopt->no_index)
+ if (!DIFF_OPT_TST(diffopt, NO_INDEX))
diffopt->skip_stat_unmatch++;
diff_free_filepair(p);
}
@@ -3095,10 +3089,10 @@ static void diffcore_skip_stat_unmatch(struct diff_options *diffopt)
void diffcore_std(struct diff_options *options)
{
- if (options->quiet)
+ if (DIFF_OPT_TST(options, QUIET))
return;
- if (options->skip_stat_unmatch && !options->find_copies_harder)
+ if (options->skip_stat_unmatch && !DIFF_OPT_TST(options, FIND_COPIES_HARDER))
diffcore_skip_stat_unmatch(options);
if (options->break_opt != -1)
diffcore_break(options->break_opt);
@@ -3113,7 +3107,10 @@ void diffcore_std(struct diff_options *options)
diff_resolve_rename_copy();
diffcore_apply_filter(options->filter);
- options->has_changes = !!diff_queued_diff.nr;
+ if (diff_queued_diff.nr)
+ DIFF_OPT_SET(options, HAS_CHANGES);
+ else
+ DIFF_OPT_CLR(options, HAS_CHANGES);
}
@@ -3137,7 +3134,7 @@ void diff_addremove(struct diff_options *options,
* Before the final output happens, they are pruned after
* merged into rename/copy pairs as appropriate.
*/
- if (options->reverse_diff)
+ if (DIFF_OPT_TST(options, REVERSE_DIFF))
addremove = (addremove == '+' ? '-' :
addremove == '-' ? '+' : addremove);
@@ -3152,7 +3149,7 @@ void diff_addremove(struct diff_options *options,
fill_filespec(two, sha1, mode);
diff_queue(&diff_queued_diff, one, two);
- options->has_changes = 1;
+ DIFF_OPT_SET(options, HAS_CHANGES);
}
void diff_change(struct diff_options *options,
@@ -3164,7 +3161,7 @@ void diff_change(struct diff_options *options,
char concatpath[PATH_MAX];
struct diff_filespec *one, *two;
- if (options->reverse_diff) {
+ if (DIFF_OPT_TST(options, REVERSE_DIFF)) {
unsigned tmp;
const unsigned char *tmp_c;
tmp = old_mode; old_mode = new_mode; new_mode = tmp;
@@ -3178,7 +3175,7 @@ void diff_change(struct diff_options *options,
fill_filespec(two, new_sha1, new_mode);
diff_queue(&diff_queued_diff, one, two);
- options->has_changes = 1;
+ DIFF_OPT_SET(options, HAS_CHANGES);
}
void diff_unmerge(struct diff_options *options,
diff --git a/diff.h b/diff.h
index 4546aad..6ff2b0e 100644
--- a/diff.h
+++ b/diff.h
@@ -43,26 +43,32 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
#define DIFF_FORMAT_CALLBACK 0x1000
+#define DIFF_OPT_RECURSIVE (1 << 0)
+#define DIFF_OPT_TREE_IN_RECURSIVE (1 << 1)
+#define DIFF_OPT_BINARY (1 << 2)
+#define DIFF_OPT_TEXT (1 << 3)
+#define DIFF_OPT_FULL_INDEX (1 << 4)
+#define DIFF_OPT_SILENT_ON_REMOVE (1 << 5)
+#define DIFF_OPT_FIND_COPIES_HARDER (1 << 6)
+#define DIFF_OPT_FOLLOW_RENAMES (1 << 7)
+#define DIFF_OPT_COLOR_DIFF (1 << 8)
+#define DIFF_OPT_COLOR_DIFF_WORDS (1 << 9)
+#define DIFF_OPT_HAS_CHANGES (1 << 10)
+#define DIFF_OPT_QUIET (1 << 11)
+#define DIFF_OPT_NO_INDEX (1 << 12)
+#define DIFF_OPT_ALLOW_EXTERNAL (1 << 13)
+#define DIFF_OPT_EXIT_WITH_STATUS (1 << 14)
+#define DIFF_OPT_REVERSE_DIFF (1 << 15)
+#define DIFF_OPT_TST(opts, flag) ((opts)->flags & DIFF_OPT_##flag)
+#define DIFF_OPT_SET(opts, flag) ((opts)->flags |= DIFF_OPT_##flag)
+#define DIFF_OPT_CLR(opts, flag) ((opts)->flags &= ~DIFF_OPT_##flag)
+
struct diff_options {
const char *filter;
const char *orderfile;
const char *pickaxe;
const char *single_follow;
- unsigned recursive:1,
- tree_in_recursive:1,
- binary:1,
- text:1,
- full_index:1,
- silent_on_remove:1,
- find_copies_harder:1,
- follow_renames:1,
- color_diff:1,
- color_diff_words:1,
- has_changes:1,
- quiet:1,
- no_index:1,
- allow_external:1,
- exit_with_status:1;
+ unsigned flags;
int context;
int break_opt;
int detect_rename;
@@ -71,7 +77,6 @@ struct diff_options {
int output_format;
int pickaxe_opts;
int rename_score;
- int reverse_diff;
int rename_limit;
int setup;
int abbrev;
@@ -105,6 +110,9 @@ enum color_diff {
DIFF_WHITESPACE = 7,
};
const char *diff_get_color(int diff_use_color, enum color_diff ix);
+#define diff_get_color_opt(o, ix) \
+ diff_get_color(DIFF_OPT_TST((o), COLOR_DIFF), ix)
+
extern const char mime_boundary_leader[];
diff --git a/log-tree.c b/log-tree.c
index a34beb0..1f3fcf1 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -245,8 +245,7 @@ void show_log(struct rev_info *opt, const char *sep)
opt->diffopt.stat_sep = buffer;
}
} else if (opt->commit_format != CMIT_FMT_USERFORMAT) {
- fputs(diff_get_color(opt->diffopt.color_diff, DIFF_COMMIT),
- stdout);
+ fputs(diff_get_color_opt(&opt->diffopt, DIFF_COMMIT), stdout);
if (opt->commit_format != CMIT_FMT_ONELINE)
fputs("commit ", stdout);
if (commit->object.flags & BOUNDARY)
@@ -266,8 +265,7 @@ void show_log(struct rev_info *opt, const char *sep)
diff_unique_abbrev(parent->object.sha1,
abbrev_commit));
show_decorations(commit);
- printf("%s",
- diff_get_color(opt->diffopt.color_diff, DIFF_RESET));
+ printf("%s", diff_get_color_opt(&opt->diffopt, DIFF_RESET));
putchar(opt->commit_format == CMIT_FMT_ONELINE ? ' ' : '\n');
if (opt->reflog_info) {
show_reflog_message(opt->reflog_info,
diff --git a/merge-recursive.c b/merge-recursive.c
index 6c6f595..9a1e2f2 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -366,7 +366,7 @@ static struct path_list *get_renames(struct tree *tree,
renames = xcalloc(1, sizeof(struct path_list));
diff_setup(&opts);
- opts.recursive = 1;
+ DIFF_OPT_SET(&opts, RECURSIVE);
opts.detect_rename = DIFF_DETECT_RENAME;
opts.rename_limit = rename_limit;
opts.output_format = DIFF_FORMAT_NO_OUTPUT;
diff --git a/patch-ids.c b/patch-ids.c
index a288fac..3be5d31 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -121,7 +121,7 @@ int init_patch_ids(struct patch_ids *ids)
{
memset(ids, 0, sizeof(*ids));
diff_setup(&ids->diffopts);
- ids->diffopts.recursive = 1;
+ DIFF_OPT_SET(&ids->diffopts, RECURSIVE);
if (diff_setup_done(&ids->diffopts) < 0)
return error("diff_setup_done failed");
return 0;
diff --git a/revision.c b/revision.c
index 931f978..040214f 100644
--- a/revision.c
+++ b/revision.c
@@ -252,7 +252,7 @@ static void file_add_remove(struct diff_options *options,
}
tree_difference = diff;
if (tree_difference == REV_TREE_DIFFERENT)
- options->has_changes = 1;
+ DIFF_OPT_SET(options, HAS_CHANGES);
}
static void file_change(struct diff_options *options,
@@ -262,7 +262,7 @@ static void file_change(struct diff_options *options,
const char *base, const char *path)
{
tree_difference = REV_TREE_DIFFERENT;
- options->has_changes = 1;
+ DIFF_OPT_SET(options, HAS_CHANGES);
}
static int rev_compare_tree(struct rev_info *revs, struct tree *t1, struct tree *t2)
@@ -272,7 +272,7 @@ static int rev_compare_tree(struct rev_info *revs, struct tree *t1, struct tree
if (!t2)
return REV_TREE_DIFFERENT;
tree_difference = REV_TREE_SAME;
- revs->pruning.has_changes = 0;
+ DIFF_OPT_CLR(&revs->pruning, HAS_CHANGES);
if (diff_tree_sha1(t1->object.sha1, t2->object.sha1, "",
&revs->pruning) < 0)
return REV_TREE_DIFFERENT;
@@ -296,7 +296,7 @@ static int rev_same_tree_as_empty(struct rev_info *revs, struct tree *t1)
init_tree_desc(&empty, "", 0);
tree_difference = REV_TREE_SAME;
- revs->pruning.has_changes = 0;
+ DIFF_OPT_CLR(&revs->pruning, HAS_CHANGES);
retval = diff_tree(&empty, &real, "", &revs->pruning);
free(tree);
@@ -688,8 +688,8 @@ void init_revisions(struct rev_info *revs, const char *prefix)
revs->abbrev = DEFAULT_ABBREV;
revs->ignore_merges = 1;
revs->simplify_history = 1;
- revs->pruning.recursive = 1;
- revs->pruning.quiet = 1;
+ DIFF_OPT_SET(&revs->pruning, RECURSIVE);
+ DIFF_OPT_SET(&revs->pruning, QUIET);
revs->pruning.add_remove = file_add_remove;
revs->pruning.change = file_change;
revs->lifo = 1;
@@ -1086,13 +1086,13 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
}
if (!strcmp(arg, "-r")) {
revs->diff = 1;
- revs->diffopt.recursive = 1;
+ DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
continue;
}
if (!strcmp(arg, "-t")) {
revs->diff = 1;
- revs->diffopt.recursive = 1;
- revs->diffopt.tree_in_recursive = 1;
+ DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
+ DIFF_OPT_SET(&revs->diffopt, TREE_IN_RECURSIVE);
continue;
}
if (!strcmp(arg, "-m")) {
@@ -1274,7 +1274,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
revs->diff = 1;
/* Pickaxe and rename following needs diffs */
- if (revs->diffopt.pickaxe || revs->diffopt.follow_renames)
+ if (revs->diffopt.pickaxe || DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))
revs->diff = 1;
if (revs->topo_order)
@@ -1283,7 +1283,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
if (revs->prune_data) {
diff_tree_setup_paths(revs->prune_data, &revs->pruning);
/* Can't prune commits with rename following: the paths change.. */
- if (!revs->diffopt.follow_renames)
+ if (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))
revs->prune = 1;
if (!revs->full_diff)
diff_tree_setup_paths(revs->prune_data, &revs->diffopt);
diff --git a/tree-diff.c b/tree-diff.c
index 7c261fd..aa0a100 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -39,7 +39,7 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const
show_entry(opt, "+", t2, base, baselen);
return 1;
}
- if (!opt->find_copies_harder && !hashcmp(sha1, sha2) && mode1 == mode2)
+ if (!DIFF_OPT_TST(opt, FIND_COPIES_HARDER) && !hashcmp(sha1, sha2) && mode1 == mode2)
return 0;
/*
@@ -52,10 +52,10 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const
return 0;
}
- if (opt->recursive && S_ISDIR(mode1)) {
+ if (DIFF_OPT_TST(opt, RECURSIVE) && S_ISDIR(mode1)) {
int retval;
char *newbase = malloc_base(base, baselen, path1, pathlen1);
- if (opt->tree_in_recursive)
+ if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE))
opt->change(opt, mode1, mode2,
sha1, sha2, base, path1);
retval = diff_tree_sha1(sha1, sha2, newbase, opt);
@@ -206,7 +206,7 @@ static void show_entry(struct diff_options *opt, const char *prefix, struct tree
const char *path;
const unsigned char *sha1 = tree_entry_extract(desc, &path, &mode);
- if (opt->recursive && S_ISDIR(mode)) {
+ if (DIFF_OPT_TST(opt, RECURSIVE) && S_ISDIR(mode)) {
enum object_type type;
int pathlen = tree_entry_len(path, sha1);
char *newbase = malloc_base(base, baselen, path, pathlen);
@@ -257,7 +257,7 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2, const char *base, stru
int baselen = strlen(base);
for (;;) {
- if (opt->quiet && opt->has_changes)
+ if (DIFF_OPT_TST(opt, QUIET) && DIFF_OPT_TST(opt, HAS_CHANGES))
break;
if (opt->nr_paths) {
skip_uninteresting(t1, base, baselen, opt);
@@ -315,7 +315,7 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co
q->nr = 0;
diff_setup(&diff_opts);
- diff_opts.recursive = 1;
+ DIFF_OPT_SET(&diff_opts, RECURSIVE);
diff_opts.detect_rename = DIFF_DETECT_RENAME;
diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
diff_opts.single_follow = opt->paths[0];
@@ -380,7 +380,7 @@ int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const cha
init_tree_desc(&t1, tree1, size1);
init_tree_desc(&t2, tree2, size2);
retval = diff_tree(&t1, &t2, base, opt);
- if (opt->follow_renames && diff_might_be_rename()) {
+ if (DIFF_OPT_TST(opt, FOLLOW_RENAMES) && diff_might_be_rename()) {
init_tree_desc(&t1, tree1, size1);
init_tree_desc(&t2, tree2, size2);
try_to_follow_renames(&t1, &t2, base, opt);
--
1.5.3.5.1598.gdef4e
^ permalink raw reply related
* [PATCH DIFF-CLEANUP 2/2] Reorder diff_opt_parse options more logically per topics.
From: Pierre Habouzit @ 2007-11-07 10:20 UTC (permalink / raw)
To: gitster; +Cc: git, Pierre Habouzit
In-Reply-To: <1194430832-6224-7-git-send-email-madcoder@debian.org>
This is a line reordering patch _only_.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
the 10 added lines are five times a blank line and a header for the each
of the 5 options groups.
This patch is very useful as it will help checking that I didn't missed
any option while converting to parseopt, it's not coquetry.
diff.c | 116 ++++++++++++++++++++++++++++++++++-----------------------------
1 files changed, 63 insertions(+), 53 deletions(-)
diff --git a/diff.c b/diff.c
index 97c9a59..7a89959 100644
--- a/diff.c
+++ b/diff.c
@@ -2198,6 +2198,8 @@ static int diff_scoreopt_parse(const char *opt);
int diff_opt_parse(struct diff_options *options, const char **av, int ac)
{
const char *arg = av[0];
+
+ /* Output format options */
if (!strcmp(arg, "-p") || !strcmp(arg, "-u"))
options->output_format |= DIFF_FORMAT_PATCH;
else if (opt_arg(arg, 'U', "unified", &options->context))
@@ -2210,6 +2212,18 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
options->output_format |= DIFF_FORMAT_NUMSTAT;
else if (!strcmp(arg, "--shortstat"))
options->output_format |= DIFF_FORMAT_SHORTSTAT;
+ else if (!strcmp(arg, "--check"))
+ options->output_format |= DIFF_FORMAT_CHECKDIFF;
+ else if (!strcmp(arg, "--summary"))
+ options->output_format |= DIFF_FORMAT_SUMMARY;
+ else if (!strcmp(arg, "--patch-with-stat"))
+ options->output_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_DIFFSTAT;
+ else if (!strcmp(arg, "--name-only"))
+ options->output_format |= DIFF_FORMAT_NAME;
+ else if (!strcmp(arg, "--name-status"))
+ options->output_format |= DIFF_FORMAT_NAME_STATUS;
+ else if (!strcmp(arg, "-s"))
+ options->output_format |= DIFF_FORMAT_NO_OUTPUT;
else if (!prefixcmp(arg, "--stat")) {
char *end;
int width = options->stat_width;
@@ -2237,42 +2251,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
options->stat_name_width = name_width;
options->stat_width = width;
}
- else if (!strcmp(arg, "--check"))
- options->output_format |= DIFF_FORMAT_CHECKDIFF;
- else if (!strcmp(arg, "--summary"))
- options->output_format |= DIFF_FORMAT_SUMMARY;
- else if (!strcmp(arg, "--patch-with-stat"))
- options->output_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_DIFFSTAT;
- else if (!strcmp(arg, "-z"))
- options->line_termination = 0;
- else if (!prefixcmp(arg, "-l"))
- options->rename_limit = strtoul(arg+2, NULL, 10);
- else if (!strcmp(arg, "--full-index"))
- DIFF_OPT_SET(options, FULL_INDEX);
- else if (!strcmp(arg, "--binary")) {
- options->output_format |= DIFF_FORMAT_PATCH;
- DIFF_OPT_SET(options, BINARY);
- }
- else if (!strcmp(arg, "-a") || !strcmp(arg, "--text"))
- DIFF_OPT_SET(options, TEXT);
- else if (!strcmp(arg, "--name-only"))
- options->output_format |= DIFF_FORMAT_NAME;
- else if (!strcmp(arg, "--name-status"))
- options->output_format |= DIFF_FORMAT_NAME_STATUS;
- else if (!strcmp(arg, "-R"))
- DIFF_OPT_SET(options, REVERSE_DIFF);
- else if (!prefixcmp(arg, "-S"))
- options->pickaxe = arg + 2;
- else if (!strcmp(arg, "-s"))
- options->output_format |= DIFF_FORMAT_NO_OUTPUT;
- else if (!prefixcmp(arg, "-O"))
- options->orderfile = arg + 2;
- else if (!prefixcmp(arg, "--diff-filter="))
- options->filter = arg + 14;
- else if (!strcmp(arg, "--pickaxe-all"))
- options->pickaxe_opts = DIFF_PICKAXE_ALL;
- else if (!strcmp(arg, "--pickaxe-regex"))
- options->pickaxe_opts = DIFF_PICKAXE_REGEX;
+
+ /* renames options */
else if (!prefixcmp(arg, "-B")) {
if ((options->break_opt = diff_scoreopt_parse(arg)) == -1)
return -1;
@@ -2289,33 +2269,38 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
return -1;
options->detect_rename = DIFF_DETECT_COPY;
}
+ else if (!strcmp(arg, "--no-renames"))
+ options->detect_rename = 0;
+
+ /* xdiff options */
+ else if (!strcmp(arg, "-w") || !strcmp(arg, "--ignore-all-space"))
+ options->xdl_opts |= XDF_IGNORE_WHITESPACE;
+ else if (!strcmp(arg, "-b") || !strcmp(arg, "--ignore-space-change"))
+ options->xdl_opts |= XDF_IGNORE_WHITESPACE_CHANGE;
+ else if (!strcmp(arg, "--ignore-space-at-eol"))
+ options->xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;
+
+ /* flags options */
+ else if (!strcmp(arg, "--binary")) {
+ options->output_format |= DIFF_FORMAT_PATCH;
+ DIFF_OPT_SET(options, BINARY);
+ }
+ else if (!strcmp(arg, "--full-index"))
+ DIFF_OPT_SET(options, FULL_INDEX);
+ else if (!strcmp(arg, "-a") || !strcmp(arg, "--text"))
+ DIFF_OPT_SET(options, TEXT);
+ else if (!strcmp(arg, "-R"))
+ DIFF_OPT_SET(options, REVERSE_DIFF);
else if (!strcmp(arg, "--find-copies-harder"))
DIFF_OPT_SET(options, FIND_COPIES_HARDER);
else if (!strcmp(arg, "--follow"))
DIFF_OPT_SET(options, FOLLOW_RENAMES);
- else if (!strcmp(arg, "--abbrev"))
- options->abbrev = DEFAULT_ABBREV;
- else if (!prefixcmp(arg, "--abbrev=")) {
- options->abbrev = strtoul(arg + 9, NULL, 10);
- if (options->abbrev < MINIMUM_ABBREV)
- options->abbrev = MINIMUM_ABBREV;
- else if (40 < options->abbrev)
- options->abbrev = 40;
- }
else if (!strcmp(arg, "--color"))
DIFF_OPT_SET(options, COLOR_DIFF);
else if (!strcmp(arg, "--no-color"))
DIFF_OPT_CLR(options, COLOR_DIFF);
- else if (!strcmp(arg, "-w") || !strcmp(arg, "--ignore-all-space"))
- options->xdl_opts |= XDF_IGNORE_WHITESPACE;
- else if (!strcmp(arg, "-b") || !strcmp(arg, "--ignore-space-change"))
- options->xdl_opts |= XDF_IGNORE_WHITESPACE_CHANGE;
- else if (!strcmp(arg, "--ignore-space-at-eol"))
- options->xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;
else if (!strcmp(arg, "--color-words"))
options->flags |= DIFF_OPT_COLOR_DIFF | DIFF_OPT_COLOR_DIFF_WORDS;
- else if (!strcmp(arg, "--no-renames"))
- options->detect_rename = 0;
else if (!strcmp(arg, "--exit-code"))
DIFF_OPT_SET(options, EXIT_WITH_STATUS);
else if (!strcmp(arg, "--quiet"))
@@ -2324,6 +2309,31 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
DIFF_OPT_SET(options, ALLOW_EXTERNAL);
else if (!strcmp(arg, "--no-ext-diff"))
DIFF_OPT_CLR(options, ALLOW_EXTERNAL);
+
+ /* misc options */
+ else if (!strcmp(arg, "-z"))
+ options->line_termination = 0;
+ else if (!prefixcmp(arg, "-l"))
+ options->rename_limit = strtoul(arg+2, NULL, 10);
+ else if (!prefixcmp(arg, "-S"))
+ options->pickaxe = arg + 2;
+ else if (!strcmp(arg, "--pickaxe-all"))
+ options->pickaxe_opts = DIFF_PICKAXE_ALL;
+ else if (!strcmp(arg, "--pickaxe-regex"))
+ options->pickaxe_opts = DIFF_PICKAXE_REGEX;
+ else if (!prefixcmp(arg, "-O"))
+ options->orderfile = arg + 2;
+ else if (!prefixcmp(arg, "--diff-filter="))
+ options->filter = arg + 14;
+ else if (!strcmp(arg, "--abbrev"))
+ options->abbrev = DEFAULT_ABBREV;
+ else if (!prefixcmp(arg, "--abbrev=")) {
+ options->abbrev = strtoul(arg + 9, NULL, 10);
+ if (options->abbrev < MINIMUM_ABBREV)
+ options->abbrev = MINIMUM_ABBREV;
+ else if (40 < options->abbrev)
+ options->abbrev = 40;
+ }
else
return 0;
return 1;
--
1.5.3.5.1598.gdef4e
^ permalink raw reply related
* [PATCH PARSEOPT 4/4] Use OPT_BIT in builtin-pack-refs
From: Pierre Habouzit @ 2007-11-07 10:20 UTC (permalink / raw)
To: gitster; +Cc: git, Pierre Habouzit
In-Reply-To: <1194430832-6224-5-git-send-email-madcoder@debian.org>
---
builtin-pack-refs.c | 12 +++---------
1 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/builtin-pack-refs.c b/builtin-pack-refs.c
index a62f06b..1923fb1 100644
--- a/builtin-pack-refs.c
+++ b/builtin-pack-refs.c
@@ -122,19 +122,13 @@ static char const * const pack_refs_usage[] = {
int cmd_pack_refs(int argc, const char **argv, const char *prefix)
{
- int all = 0, prune = 1;
- unsigned int flags = 0;
+ unsigned int flags = PACK_REFS_PRUNE;
struct option opts[] = {
- OPT_BOOLEAN(0, "all", &all, "pack everything"),
- OPT_BOOLEAN(0, "prune", &prune, "prune loose refs (default)"),
+ OPT_BIT(0, "all", &flags, "pack everything", PACK_REFS_ALL),
+ OPT_BIT(0, "prune", &flags, "prune loose refs (default)", PACK_REFS_PRUNE),
OPT_END(),
};
-
if (parse_options(argc, argv, opts, pack_refs_usage, 0))
usage_with_options(pack_refs_usage, opts);
- if (prune)
- flags |= PACK_REFS_PRUNE;
- if (all)
- flags |= PACK_REFS_ALL;
return pack_refs(flags);
}
--
1.5.3.5.1598.gdef4e
^ permalink raw reply related
* Preliminary patches to the diff.[hc] parseoptification
From: Pierre Habouzit @ 2007-11-07 10:20 UTC (permalink / raw)
To: gitster; +Cc: git
I'm currently working on the very involved task of migrating diff
options to parseopts. It's quite involved because for many reasons I
won't detail here, it needs to migrate those _and_ revisions options at
the same time (most of the diff options users _also_ use the revisions
ones).
So I'm trying to see some preliminary patches that I need to make this
the less disruptive possible: my goal is that people can use those
preliminary series bit by bit, and not a whole 10 to 20 patches (I
believe it will really take this amount in the end) at once with
breakages hard to find in the mess.
So in the coming days I'll probably send my work bit by bit when I feel
it won't change anymore.
Summary:
~~~~~~~
The 3 series are completely independent.
+ this is a patch to fix an issue that begin to irritate me and
prevents me to build with -Werror.
[1/1] Make gcc warning about empty if body go away.
+ this series add new features that are needed for the diff options.
commit 1 implements them (see commit log,
commits 2 to 4 use them in some commands that benefit from those.
[1/4] parse-options new features.
[2/4] Use OPT_SET_INT and OPT_BIT in builtin-branch
[3/4] Use OPT_BIT in builtin-for-each-ref
[4/4] Use OPT_BIT in builtin-pack-refs
+ this series does some modifications on the diff_options structure
and the diff_opt_parse functions. Those are not fundamentally needed
without parseopt, but parseopt will need them.
Basically, diff_options had bitfields (the C :1 kind). parseopt cannot
grok that portably, so I've converted these into a single
`unsigned flags` with explicit masks to use (and helper macros to
avoid overlong lines).
The obvious goal is that I can use the OPTION_BIT feature from the
previous series to fill them from parse_options.
[1/2] Make the diff_options bitfields be an unsigned with explicit masks.
[2/2] Reorder diff_opt_parse options more logically per topics.
Those patches are available on my public repository in the
ph/parseopt-stable branch.
ph/parseopt is my WIP and has many many horrible commits right now, that
I rebase -i to cleanse them once the big picture pleases me enough,
though people may want to look at diff.h in that branch to see where it
goes. I'm pleased to say that it seems I'll only need 3 diff-specific
callbacks, which is not a lot:
http://git.madism.org/?p=git.git;a=blobdiff;f=diff.h;h=0e44e5;hp=6ff2b0;hb=def4eb;hpb=bdc1ab
I suppose that the split of this big macro is explanatory enough for the
reason behind commit 2/2 of the third series...
Diff stats:
~~~~~~~~~~
+ gcc issue:
builtin-diff.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
+ parseopt features:
builtin-branch.c | 44 ++++++++++++-----------------------
builtin-for-each-ref.c | 19 ++++++++-------
builtin-pack-refs.c | 12 ++-------
git-compat-util.h | 1
parse-options.c | 61 +++++++++++++++++++++++++++++++++++--------------
parse-options.h | 15 +++++++++++-
6 files changed, 88 insertions(+), 64 deletions(-)
+ diff-cleanup:
builtin-blame.c | 10 +-
builtin-diff-files.c | 4 +-
builtin-diff-index.c | 4 +-
builtin-diff-tree.c | 9 +-
builtin-diff.c | 12 ++--
builtin-log.c | 24 +++---
combine-diff.c | 10 +-
diff-lib.c | 23 +++---
diff.c | 221 ++++++++++++++++++++++++++------------------------
diff.h | 40 ++++++----
log-tree.c | 6 +-
merge-recursive.c | 2 +-
patch-ids.c | 2 +-
revision.c | 22 +++---
tree-diff.c | 14 ++--
15 files changed, 209 insertions(+), 194 deletions(-)
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
^ permalink raw reply
* [PATCH PARSEOPT 3/4] Use OPT_BIT in builtin-for-each-ref
From: Pierre Habouzit @ 2007-11-07 10:20 UTC (permalink / raw)
To: gitster; +Cc: git, Pierre Habouzit
In-Reply-To: <1194430832-6224-4-git-send-email-madcoder@debian.org>
---
builtin-for-each-ref.c | 19 ++++++++++---------
1 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index da8c794..1a23ea6 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -833,16 +833,19 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
int i, num_refs;
const char *format = "%(objectname) %(objecttype)\t%(refname)";
struct ref_sort *sort = NULL, **sort_tail = &sort;
- int maxcount = 0, quote_style;
- int quote_shell = 0, quote_perl = 0, quote_python = 0, quote_tcl = 0;
+ int maxcount = 0, quote_style = 0;
struct refinfo **refs;
struct grab_ref_cbdata cbdata;
struct option opts[] = {
- OPT_BOOLEAN('s', "shell", "e_shell, "quote placeholders suitably for shells"),
- OPT_BOOLEAN('p', "perl", "e_perl, "quote placeholders suitably for perl"),
- OPT_BOOLEAN( 0 , "python", "e_python, "quote placeholders suitably for python"),
- OPT_BOOLEAN( 0 , "tcl", "e_tcl, "quote placeholders suitably for tcl"),
+ OPT_BIT('s', "shell", "e_style,
+ "quote placeholders suitably for shells", QUOTE_SHELL),
+ OPT_BIT('p', "perl", "e_style,
+ "quote placeholders suitably for perl", QUOTE_PERL),
+ OPT_BIT(0 , "python", "e_style,
+ "quote placeholders suitably for python", QUOTE_PYTHON),
+ OPT_BIT(0 , "tcl", "e_style,
+ "quote placeholders suitably for tcl", QUOTE_TCL),
OPT_GROUP(""),
OPT_INTEGER( 0 , "count", &maxcount, "show only <n> matched refs"),
@@ -857,15 +860,13 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
error("invalid --count argument: `%d'", maxcount);
usage_with_options(for_each_ref_usage, opts);
}
- if (quote_shell + quote_perl + quote_python + quote_tcl > 1) {
+ if (HAS_MULTI_BITS(quote_style)) {
error("more than one quoting style ?");
usage_with_options(for_each_ref_usage, opts);
}
if (verify_format(format))
usage_with_options(for_each_ref_usage, opts);
- quote_style = QUOTE_SHELL * quote_shell + QUOTE_PERL * quote_perl +
- QUOTE_PYTHON * quote_python + QUOTE_TCL * quote_tcl;
if (!sort)
sort = default_sort();
sort_atom_limit = used_atom_cnt;
--
1.5.3.5.1598.gdef4e
^ permalink raw reply related
* [PATCH PARSEOPT 2/4] Use OPT_SET_INT and OPT_BIT in builtin-branch
From: Pierre Habouzit @ 2007-11-07 10:20 UTC (permalink / raw)
To: gitster; +Cc: git, Pierre Habouzit
In-Reply-To: <1194430832-6224-3-git-send-email-madcoder@debian.org>
Also remove a spurious after-check on --abbrev (OPT__ABBREV already takes
care of that)
---
builtin-branch.c | 44 ++++++++++++++++----------------------------
1 files changed, 16 insertions(+), 28 deletions(-)
diff --git a/builtin-branch.c b/builtin-branch.c
index 3bf40f1..2694c9c 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -507,48 +507,36 @@ static void rename_branch(const char *oldname, const char *newname, int force)
int cmd_branch(int argc, const char **argv, const char *prefix)
{
- int delete = 0, force_delete = 0, force_create = 0;
- int rename = 0, force_rename = 0;
+ int delete = 0, rename = 0, force_create = 0;
int verbose = 0, abbrev = DEFAULT_ABBREV, detached = 0;
int reflog = 0, track;
- int kinds = REF_LOCAL_BRANCH, kind_remote = 0, kind_any = 0;
+ int kinds = REF_LOCAL_BRANCH;
struct option options[] = {
OPT_GROUP("Generic options"),
OPT__VERBOSE(&verbose),
OPT_BOOLEAN( 0 , "track", &track, "set up tracking mode (see git-pull(1))"),
OPT_BOOLEAN( 0 , "color", &branch_use_color, "use colored output"),
- OPT_BOOLEAN('r', NULL, &kind_remote, "act on remote-tracking branches"),
+ OPT_SET_INT('r', NULL, &kinds, "act on remote-tracking branches",
+ REF_REMOTE_BRANCH),
OPT__ABBREV(&abbrev),
OPT_GROUP("Specific git-branch actions:"),
- OPT_BOOLEAN('a', NULL, &kind_any, "list both remote-tracking and local branches"),
- OPT_BOOLEAN('d', NULL, &delete, "delete fully merged branch"),
- OPT_BOOLEAN('D', NULL, &force_delete, "delete branch (even if not merged)"),
- OPT_BOOLEAN('l', NULL, &reflog, "create the branch's reflog"),
- OPT_BOOLEAN('f', NULL, &force_create, "force creation (when already exists)"),
- OPT_BOOLEAN('m', NULL, &rename, "move/rename a branch and its reflog"),
- OPT_BOOLEAN('M', NULL, &force_rename, "move/rename a branch, even if target exists"),
+ OPT_SET_INT('a', NULL, &kinds, "list both remote-tracking and local branches",
+ REF_REMOTE_BRANCH | REF_LOCAL_BRANCH),
+ OPT_BIT('d', NULL, &delete, "delete fully merged branch", 1),
+ OPT_BIT('D', NULL, &delete, "delete branch (even if not merged)", 2),
+ OPT_BIT('m', NULL, &rename, "move/rename a branch and its reflog", 1),
+ OPT_BIT('M', NULL, &rename, "move/rename a branch, even if target exists", 2),
+ OPT_BOOLEAN('l', NULL, &reflog, "create the branch's reflog"),
+ OPT_BOOLEAN('f', NULL, &force_create, "force creation (when already exists)"),
OPT_END(),
};
git_config(git_branch_config);
track = branch_track;
argc = parse_options(argc, argv, options, builtin_branch_usage, 0);
-
- delete |= force_delete;
- rename |= force_rename;
- if (kind_remote)
- kinds = REF_REMOTE_BRANCH;
- if (kind_any)
- kinds = REF_REMOTE_BRANCH | REF_LOCAL_BRANCH;
- if (abbrev && abbrev < MINIMUM_ABBREV)
- abbrev = MINIMUM_ABBREV;
- else if (abbrev > 40)
- abbrev = 40;
-
- if ((delete && rename) || (delete && force_create) ||
- (rename && force_create))
+ if (!!delete + !!rename + !!force_create > 1)
usage_with_options(builtin_branch_usage, options);
head = resolve_ref("HEAD", head_sha1, 0, NULL);
@@ -564,13 +552,13 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
}
if (delete)
- return delete_branches(argc, argv, force_delete, kinds);
+ return delete_branches(argc, argv, delete > 1, kinds);
else if (argc == 0)
print_ref_list(kinds, detached, verbose, abbrev);
else if (rename && (argc == 1))
- rename_branch(head, argv[0], force_rename);
+ rename_branch(head, argv[0], rename > 1);
else if (rename && (argc == 2))
- rename_branch(argv[0], argv[1], force_rename);
+ rename_branch(argv[0], argv[1], rename > 1);
else if (argc <= 2)
create_branch(argv[0], (argc == 2) ? argv[1] : head,
force_create, reflog, track);
--
1.5.3.5.1598.gdef4e
^ permalink raw reply related
* [PATCH PARSEOPT 1/4] parse-options new features.
From: Pierre Habouzit @ 2007-11-07 10:20 UTC (permalink / raw)
To: gitster; +Cc: git, Pierre Habouzit
In-Reply-To: <1194430832-6224-2-git-send-email-madcoder@debian.org>
options flags:
~~~~~~~~~~~~~
PARSE_OPT_NONEG allow the caller to disallow the negated option to exists.
option types:
~~~~~~~~~~~~
OPTION_BIT: ORs (or NANDs) a mask.
OPTION_SET_INT: force the value to be set to this integer.
OPTION_SET_PTR: force the value to be set to this pointer.
helper:
~~~~~~
HAS_MULTI_BITS (in git-compat-util.h) is a bit-hack to check if an
unsigned integer has more than one bit set, useful to check if conflicting
options have been used.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
git-compat-util.h | 1 +
parse-options.c | 61 ++++++++++++++++++++++++++++++++++++++--------------
parse-options.h | 15 ++++++++++++-
3 files changed, 59 insertions(+), 18 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index 7b29d1b..f86b19f 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -20,6 +20,7 @@
#endif
#define MSB(x, bits) ((x) & TYPEOF(x)(~0ULL << (sizeof(x) * 8 - (bits))))
+#define HAS_MULTI_BITS(i) ((i) & ((i) - 1)) /* checks if an integer has more than 1 bit set */
/* Approximation of the length of the decimal representation of this type. */
#define decimal_length(x) ((int)(sizeof(x) * 2.56 + 0.5) + 1)
diff --git a/parse-options.c b/parse-options.c
index 15b32f7..d3e608a 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -40,24 +40,53 @@ static int get_value(struct optparse_t *p,
const struct option *opt, int flags)
{
const char *s, *arg;
- arg = p->opt ? p->opt : (p->argc > 1 ? p->argv[1] : NULL);
+ const int unset = flags & OPT_UNSET;
- if (p->opt && (flags & OPT_UNSET))
+ if (unset && p->opt)
return opterror(opt, "takes no value", flags);
+ if (unset && (opt->flags & PARSE_OPT_NONEG))
+ return opterror(opt, "isn't available", flags);
- switch (opt->type) {
- case OPTION_BOOLEAN:
- if (!(flags & OPT_SHORT) && p->opt)
+ if (!(flags & OPT_SHORT) && p->opt) {
+ switch (opt->type) {
+ case OPTION_CALLBACK:
+ if (!(opt->flags & PARSE_OPT_NOARG))
+ break;
+ /* FALLTHROUGH */
+ case OPTION_BOOLEAN:
+ case OPTION_BIT:
+ case OPTION_SET_INT:
+ case OPTION_SET_PTR:
return opterror(opt, "takes no value", flags);
- if (flags & OPT_UNSET)
- *(int *)opt->value = 0;
+ default:
+ break;
+ }
+ }
+
+ arg = p->opt ? p->opt : (p->argc > 1 ? p->argv[1] : NULL);
+ switch (opt->type) {
+ case OPTION_BIT:
+ if (unset)
+ *(int *)opt->value &= ~opt->defval;
else
- (*(int *)opt->value)++;
+ *(int *)opt->value |= opt->defval;
+ return 0;
+
+ case OPTION_BOOLEAN:
+ *(int *)opt->value = unset ? 0 : *(int *)opt->value + 1;
+ return 0;
+
+ case OPTION_SET_INT:
+ *(int *)opt->value = unset ? 0 : opt->defval;
+ return 0;
+
+ case OPTION_SET_PTR:
+ *(void **)opt->value = unset ? NULL : (void *)opt->defval;
return 0;
case OPTION_STRING:
- if (flags & OPT_UNSET) {
- *(const char **)opt->value = (const char *)NULL;
+ if (unset) {
+ *(const char **)opt->value = NULL;
return 0;
}
if (opt->flags & PARSE_OPT_OPTARG && (!arg || *arg == '-')) {
@@ -70,13 +99,10 @@ static int get_value(struct optparse_t *p,
return 0;
case OPTION_CALLBACK:
- if (flags & OPT_UNSET)
+ if (unset)
return (*opt->callback)(opt, NULL, 1);
- if (opt->flags & PARSE_OPT_NOARG) {
- if (p->opt && !(flags & OPT_SHORT))
- return opterror(opt, "takes no value", flags);
+ if (opt->flags & PARSE_OPT_NOARG)
return (*opt->callback)(opt, NULL, 0);
- }
if (opt->flags & PARSE_OPT_OPTARG && (!arg || *arg == '-'))
return (*opt->callback)(opt, NULL, 0);
if (!arg)
@@ -84,7 +110,7 @@ static int get_value(struct optparse_t *p,
return (*opt->callback)(opt, get_arg(p), 0);
case OPTION_INTEGER:
- if (flags & OPT_UNSET) {
+ if (unset) {
*(int *)opt->value = 0;
return 0;
}
@@ -292,7 +318,7 @@ void usage_with_options(const char * const *usagestr,
pos += fprintf(stderr, " ...");
}
break;
- default:
+ default: /* OPTION_{BIT,BOOLEAN,SET_INT,SET_PTR} */
break;
}
@@ -311,6 +337,7 @@ void usage_with_options(const char * const *usagestr,
/*----- some often used options -----*/
#include "cache.h"
+
int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset)
{
int v;
diff --git a/parse-options.h b/parse-options.h
index 65bce6e..a8760ac 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -2,9 +2,15 @@
#define PARSE_OPTIONS_H
enum parse_opt_type {
+ /* special types */
OPTION_END,
OPTION_GROUP,
- OPTION_BOOLEAN,
+ /* options with no arguments */
+ OPTION_BIT,
+ OPTION_BOOLEAN, /* _INCR would have been a better name */
+ OPTION_SET_INT,
+ OPTION_SET_PTR,
+ /* options with arguments (usually) */
OPTION_STRING,
OPTION_INTEGER,
OPTION_CALLBACK,
@@ -17,6 +23,7 @@ enum parse_opt_flags {
enum parse_opt_option_flags {
PARSE_OPT_OPTARG = 1,
PARSE_OPT_NOARG = 2,
+ PARSE_OPT_NONEG = 4,
};
struct option;
@@ -49,12 +56,15 @@ typedef int parse_opt_cb(const struct option *, const char *arg, int unset);
* mask of parse_opt_option_flags.
* PARSE_OPT_OPTARG: says that the argument is optionnal (not for BOOLEANs)
* PARSE_OPT_NOARG: says that this option takes no argument, for CALLBACKs
+ * PARSE_OPT_NONEG: says that this option cannot be negated
*
* `callback`::
* pointer to the callback to use for OPTION_CALLBACK.
*
* `defval`::
* default value to fill (*->value) with for PARSE_OPT_OPTARG.
+ * OPTION_{BIT,SET_INT,SET_PTR} store the {mask,integer,pointer} to put in
+ * the value when met.
* CALLBACKS can use it like they want.
*/
struct option {
@@ -72,7 +82,10 @@ struct option {
#define OPT_END() { OPTION_END }
#define OPT_GROUP(h) { OPTION_GROUP, 0, NULL, NULL, NULL, (h) }
+#define OPT_BIT(s, l, v, h, b) { OPTION_BIT, (s), (l), (v), NULL, (h), 0, NULL, (b) }
#define OPT_BOOLEAN(s, l, v, h) { OPTION_BOOLEAN, (s), (l), (v), NULL, (h) }
+#define OPT_SET_INT(s, l, v, h, i) { OPTION_SET_INT, (s), (l), (v), NULL, (h), 0, NULL, (i) }
+#define OPT_SET_PTR(s, l, v, h, p) { OPTION_SET_PTR, (s), (l), (v), NULL, (h), 0, NULL, (p) }
#define OPT_INTEGER(s, l, v, h) { OPTION_INTEGER, (s), (l), (v), NULL, (h) }
#define OPT_STRING(s, l, v, a, h) { OPTION_STRING, (s), (l), (v), (a), (h) }
#define OPT_CALLBACK(s, l, v, a, h, f) \
--
1.5.3.5.1598.gdef4e
^ permalink raw reply related
* [PATCH MISC 1/1] Make gcc warning about empty if body go away.
From: Pierre Habouzit @ 2007-11-07 10:20 UTC (permalink / raw)
To: gitster; +Cc: git, Pierre Habouzit
In-Reply-To: <1194430832-6224-1-git-send-email-madcoder@debian.org>
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
builtin-diff.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin-diff.c b/builtin-diff.c
index f77352b..80392a8 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -204,7 +204,7 @@ static void refresh_index_quietly(void)
if (write_cache(fd, active_cache, active_nr) ||
close(fd) ||
commit_locked_index(lock_file))
- ; /*
+ (void)0; /*
* silently ignore it -- we haven't mucked
* with the real index.
*/
--
1.5.3.5.1598.gdef4e
^ permalink raw reply related
* Re: [PATCH] Documentation: enhanced "git for CVS users" doc about shared repositories
From: David Kastrup @ 2007-11-07 9:09 UTC (permalink / raw)
To: git
In-Reply-To: <Pine.LNX.4.64.0711070053320.4362@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Remember, those who read "git for CVS users" are _unwilling_ to
> spend the time reading git documentation (at least for the most
> part). If they encounter something which is not useful to them,
> they will not just ignore it, they will stop reading.
People who read documentation do this only in order to avoid reading
documentation, anyway? This must be about the most stupid argument to
refrain from augmenting documentation I have heard in a long time.
It may well be possible that some of the proposed additions don't have
a good place in the document. I have not checked so myself. But the
above criticism is not constructive for documentation writers since
its logical conclusion would boil down to "don't bother writing any
documentation".
--
David Kastrup
^ permalink raw reply
* Re: [PATCH] git-revert is one of the most misunderstood command in git, help users out.
From: David Kastrup @ 2007-11-07 9:03 UTC (permalink / raw)
To: git
In-Reply-To: <Pine.LNX.4.64.0711062225090.4362@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Tue, 6 Nov 2007, Robin Rosenberg wrote:
>
>> tisdag 06 november 2007 skrev Mike Hommey:
>> > Maybe the documentation could emphasise on how to undo things when the
>> > user makes mistakes.
>> > Sometimes, saving your repo can be as simple as git reset --hard HEAD@{1}.
>> > This is not, unfortunately, a works-for-all-cases command.
>>
>> Yea, git-undo(7).
>
> In related news, I know a few users who need an un-rm-rf. Anyone?
Most file systems don't have a reflog or other ways to recover from
shooting yourself in the foot. git has, and for good reason.
There is no sense in hiding that facility away because of feeling
macho. Since git already keeps the file space around needed for
recovery (and you really have to exert yourself to make it let go for
good), there is no point in not making it as convenient as feasible to
recover.
--
David Kastrup
^ permalink raw reply
* Re: [PATCH] Add Documentation/CodingStyle
From: Andreas Ericsson @ 2007-11-07 8:53 UTC (permalink / raw)
To: Wincent Colaiuta
Cc: Johannes Schindelin, Junio C Hamano, Ralf Wildenhues, git
In-Reply-To: <4749D87D-9660-472B-97CF-8E649435AFD7@wincent.com>
Wincent Colaiuta wrote:
> El 7/11/2007, a las 0:17, Johannes Schindelin escribió:
>
>> Even if our code is quite a good documentation for our coding style,
>> some people seem to prefer a document describing it.
>
> Great idea, Johannes, especially your nice concise summary of
> conventions for shell-scripts (which is an area where we most often see
> list traffic on which way to write things).
>
That was ripped from a mail Junio sent to the list though ;-)
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH] Add Documentation/CodingStyle
From: Andreas Ericsson @ 2007-11-07 8:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Ralf Wildenhues, git
In-Reply-To: <7v640ega5q.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
>> new file mode 100644
>> index 0000000..622b80b
>> --- /dev/null
>> +++ b/Documentation/CodingStyle
>> @@ -0,0 +1,87 @@
>> +As a popular project, we also have some guidelines to keep to the
>> +code. For git in general, two rough rules are:
>> +
>> + - Most importantly, we never say "It's in POSIX; we'll happily
>> + screw your system that does not conform." We live in the
>> + real world.
>> +
>> + - However, we often say "Let's stay away from that construct,
>> + it's not even in POSIX".
>> +
>
> I am not sure if we want to have CodingStyle document, but the
> above are not CodingStyle issues.
>
> If we are to write this down, I'd like to have the more
> important third rule, which is:
>
> - In spite of the above two rules, we sometimes say "Although
> this is not in POSIX, it (is so convenient | makes the code
> much more readable | has other good characteristics) and
> practically all the platforms we care about support it, so
> let's use it". Again, we live in the real world, and it is
> sometimes a judgement call, decided based more on real world
> constraints people face than what the paper standard says.
>
>> +For C programs:
>> +
>> + - Use tabs to increment, and interpret tabs as taking up to 8 spaces
>
> What's the character for decrement? DEL? ;-)
>
>> + Double negation is often harder to understand than no negation at
>> + all.
>> +
>> + Some clever tricks, like using the !! operator with arithmetic
>> + constructs, can be extremely confusing to others. Avoid them,
>> + unless there is a compelling reason to use them.
>
> I actually think (!!var) idiom is already established in our
> codebase.
>
Not very widely for arithmetic expressions though. I believe this
alludes to the (!!a + !!b + !!c) idiom used earlier, where it's not
exactly clear *why* a, b and c can be > 1 if != 0 is the only thing
we care about.
>> + - Use the API. No, really. We have a strbuf (variable length string),
>> + several arrays with the ALLOC_GROW() macro, a path_list for sorted
>> + string lists, a hash map (mapping struct objects) named
>> + "struct decorate", amongst other things.
>
> - When you come up with an API, document it.
>
>> + - if you are planning a new command, consider writing it in shell or
>> + perl first, so that changes in semantics can be easily changed and
>> + discussed. Many git commands started out like that, and a few are
>> + still scripts.
>
> No Python allowed?
Perhaps with this addendum?
- Think very, very hard before introducing a new dependency into git. This
means you should stay away from scripting languages not already used in
the git core command set unless your command is clearly separate from it,
such as an importer to convert random-scm-X repositories to git.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ 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