Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> @@ -3758,28 +3758,25 @@ static void mark_bitmap_preferred_tips(void)
> static enum rev_info_stdin_line get_object_list_handle_stdin_line(
> struct rev_info *revs, struct strbuf *line_sb, void *stdin_line_priv)
> {
> - int *flags = stdin_line_priv;
> char *line = line_sb->buf;
>
> - if (*line == '-') {
> - if (!strcmp(line, "--not")) {
> - *flags ^= UNINTERESTING;
> - write_bitmap_index = 0;
> - return REV_INFO_STDIN_LINE_CONTINUE;
> - }
> - if (starts_with(line, "--shallow ")) {
> - struct object_id oid;
> - if (get_oid_hex(line + 10, &oid))
> - die("not an object name '%s'", line + 10);
> - register_shallow(the_repository, &oid);
> - use_bitmap_index = 0;
> - return REV_INFO_STDIN_LINE_CONTINUE;
> - }
> + if (*line != '-')
> + return REV_INFO_STDIN_LINE_PROCESS;
> +
> + if (!strcmp(line, "--not")) {
> + revs->revarg_flags ^= UNINTERESTING;
> + write_bitmap_index = 0;
> + return REV_INFO_STDIN_LINE_CONTINUE;
> + } else if (starts_with(line, "--shallow ")) {
> + struct object_id oid;
> + if (get_oid_hex(line + 10, &oid))
> + die("not an object name '%s'", line + 10);
> + register_shallow(the_repository, &oid);
> + use_bitmap_index = 0;
> + return REV_INFO_STDIN_LINE_CONTINUE;
> + } else {
> die(_("not a rev '%s'"), line);
> }
> - if (handle_revision_arg(line, revs, *flags, REVARG_CANNOT_BE_FILENAME))
> - die(_("bad revision '%s'"), line);
> - return REV_INFO_STDIN_LINE_CONTINUE;
> }
Now, this does make things slightly more modular (specifically, it
no longer is necessary for handle_revision_arg() to be visible
outside revision.c).
Having said that, it is not immediately obvious if the amount of
code churn by 3/4 and 4/4 is justified. This still looks more like
"we can add a new API, so we added it" than "we needed a new API for
such and such reasons---now we can do this more easily then before".
It's not wrong per-se, but I am not all that impressed. Sorry.
