Re: [PATCH 3/3] git-commit.sh: convert run_status to a C builtin
- From
Junio C Hamano <junkio@cox.net>
- Date
- Sep 8, 2006, 00:20 UTC
- Message-ID
- <7vzmdbqke3.fsf@assigned-by-dhcp.cox.net>
- In-Reply-To
- <20060907063621.GC17083@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 13 quoted lines
> diff --git a/Makefile b/Makefile > index 78748cb..d0ba3b5 100644 > --- a/Makefile > +++ b/Makefile > @@ -252,7 +252,7 @@ LIB_OBJS = \ > fetch-clone.o revision.o pager.o tree-walk.o xdiff-interface.o \ > write_or_die.o trace.o \ > alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \ > - color.o > + color.o status.o > > BUILTIN_OBJS = \ > builtin-add.o \
At some point (this does not have to be part of this series), we should demote DIFF_OBJS to just ordinary LIB_OBJS. They used to be special in that they are used only by handful special commands, but these days more and more things are integrated and it outlived its usefulness.
Show 29 quoted lines
> +static const char runstatus_usage[] =
> +"git-runstatus [--color|--nocolor] [--amend] [--verbose]";
> +
> +int cmd_runstatus(int argc, const char **argv, const char *prefix)
> +{
> + struct status s;
> + int i;
> +
> + git_config(git_status_config);
> + status_prepare(&s);
> +
> + for (i = 1; i < argc; i++) {
> + if (!strcmp(argv[i], "--color"))
> + status_use_color = 1;
> + else if (!strcmp(argv[i], "--nocolor"))
> + status_use_color = 0;
> + else if (!strcmp(argv[i], "--amend")) {
> + s.amend = 1;
> + s.reference = "HEAD^1";
> + }
> + else if (!strcmp(argv[i], "--verbose"))
> + s.verbose = 1;
> + else
> + usage(runstatus_usage);
> + }
> +
> + status_print(&s);
> + return s.commitable ? 0 : 1;
> +}Quite funny indentation style your MUA likes ;-).
Show 7 quoted lines
> diff --git a/status.c b/status.c > new file mode 100644 > index 0000000..82aa881 > --- /dev/null > +++ b/status.c > @@ -0,0 +1,283 @@ > +#include "status.h"
"status.h" and "struct status" somehow sounds too broad. Granted, "object.h" is also broad, but in git context "object" has a specific meaning.
Having said that I cannot come up with a good alternative name. It is not "project status" nor "repository status". It is "working tree status", but that sounds very loooooooooooooooong.
> + color_printf(color(STATUS_HEADER), "#\t");
> + switch (p->status) {
> + case DIFF_STATUS_ADDED:
> + color_printf(c, "new file: %s\n", p->one->path); break;Very nicely done. Especially I liked that you are careful not to paint leading '#\t' (which is noticeable when you use reverse as an attribute).
Show 41 quoted lines
> +static void
> +status_print_untracked(const struct status *s)
> +{
> + struct dir_struct dir;
> + const char *x;
> + int i;
> + int shown_header = 0;
> +
> + memset(&dir, 0, sizeof(dir));
> +
> + dir.exclude_per_dir = ".gitignore";
> + x = git_path("info/exclude");
> + if (file_exists(x))
> + add_excludes_from_file(&dir, x);
> +
> + read_directory(&dir, ".", "", 0);
> + for(i = 0; i < dir.nr; i++) {
> + /* check for matching entry, which is unmerged; lifted from
> + * builtin-ls-files:show_other_files */
> + struct dir_entry *ent = dir.entries[i];
> + int pos = cache_name_pos(ent->name, ent->len);
> + struct cache_entry *ce;
> + if (0 <= pos)
> + die("bug in status_print_untracked");
> + pos = -pos - 1;
> + if (pos < active_nr) {
> + ce = active_cache[pos];
> + if (ce_namelen(ce) == ent->len &&
> + !memcmp(ce->name, ent->name, ent->len))
> + continue;
> + }
> + if (!shown_header) {
> + status_print_header("Untracked files",
> + "use \"git add\" to add to commit");
> + shown_header = 1;
> + }
> + color_printf(color(STATUS_HEADER), "#\t");
> + color_printf(color(STATUS_UNTRACKED), "%.*s\n",
> + ent->len, ent->name);
> + }
> +}Very nice code reuse. I do not mean sarcasm -- the part copied and pasted from ls-files is almost trivial to bother factoring out. What's nice is read_directory() does all what is needed to deal with .gitignore files, which I forgot almost all about.
> +int status_foreach_cached(status_cb cb); > +int status_foreach_updated(status_cb cb); > +int status_foreach_changed(status_cb cb); > +int status_foreach_untracked(status_cb cb);
I do not see them defined nor used...
I'll take only [1/3] for now but I am interested in 2 and 3.