mbox series

[00/76] Convert diff opt parser to parse_options()

Message ID 20190117130615.18732-1-pclouds@gmail.com (mailing list archive)
Headers show
Series Convert diff opt parser to parse_options() | expand

Message

Duy Nguyen Jan. 17, 2019, 1:04 p.m. UTC 
This series converts diff option parsing to using parse-options. There
are a couple benefits of using parse-options, including "git <cmd> -h"
output, completion and less duplicate work.

This is the first half. The second one would be converting the option
parser in revision.c.  After that, the end game is, any command can
take a 'struct option[]' somewhere from diff/rev code, remove the
options they are not interested, then merge with their own options and
do parse_options() just once. There will be no separate parse phase
for revision/diff anymore.

I sent a sneak peek [1] last year and got two good comments. I take it
people at least did not oppose to this. The most interesting parts are
at the top and bottom. The middle is just boring conversion, usually
one option per patch.

This will conflict with pw/diff-color-moved-ws-fix on 'pu' because
that series adds --no-color-moved-ws option. I redid the same thing in
73/76 so that conflict resolution could be simply taking this side of
change.

This series is also available for fetching here

https://gitlab.com/pclouds/git/commits/diff-opt-parse-options

[1] http://public-inbox.org/git/20181227162536.15895-1-pclouds@gmail.com/

Nguyễn Thái Ngọc Duy (76):
  parse-options.h: remove extern on function prototypes
  parse-options: add one-shot mode
  parse-options: allow keep-unknown + stop-at-non-opt combination
  parse-options: disable option abbreviation with PARSE_OPT_KEEP_UNKNOWN
  parse-options: add OPT_BITOP()
  parse-options: stop abusing 'callback' for lowlevel callbacks
  parse-options: avoid magic return codes
  parse-options: allow ll_callback with OPTION_CALLBACK
  diff.h: keep forward struct declarations sorted
  diff.h: avoid bit fields in struct diff_flags
  diff.c: prepare to use parse_options() for parsing
  diff.c: convert -u|-p|--patch
  diff.c: convert -U|--unified
  diff.c: convert -W|--[no-]function-context
  diff.c: convert --raw
  diff.c: convert --patch-with-raw
  diff.c: convert --numstat and --shortstat
  diff.c: convert --dirstat and friends
  diff.c: convert --check
  diff.c: convert --summary
  diff.c: convert --patch-with-stat
  diff.c: convert --name-only
  diff.c: convert --name-status
  diff.c: convert -s|--no-patch
  diff.c: convert --stat*
  diff.c: convert --[no-]compact-summary
  diff.c: convert --output-*
  diff.c: convert -B|--break-rewrites
  diff.c: convert -M|--find-renames
  diff.c: convert -D|--irreversible-delete
  diff.c: convert -C|--find-copies
  diff.c: convert --find-copies-harder
  diff.c: convert --no-renames|--[no--rename-empty
  diff.c: convert --relative
  diff.c: convert --[no-]minimal
  diff.c: convert --ignore-some-changes
  diff.c: convert --[no-]indent-heuristic
  diff.c: convert --patience
  diff.c: convert --histogram
  diff.c: convert --diff-algorithm
  diff.c: convert --anchored
  diff.c: convert --binary
  diff.c: convert --full-index
  diff.c: convert -a|--text
  diff.c: convert -R
  diff.c: convert --[no-]follow
  diff.c: convert --[no-]color
  diff.c: convert --word-diff
  diff.c: convert --word-diff-regex
  diff.c: convert --color-words
  diff.c: convert --exit-code
  diff.c: convert --quiet
  diff.c: convert --ext-diff
  diff.c: convert --textconv
  diff.c: convert --ignore-submodules
  diff.c: convert --submodule
  diff.c: convert --ws-error-highlight
  diff.c: convert --ita-[in]visible-in-index
  diff.c: convert -z
  diff.c: convert -l
  diff.c: convert -S|-G
  diff.c: convert --pickaxe-all|--pickaxe-regex
  diff.c: convert -O
  diff.c: convert --find-object
  diff.c: convert --diff-filter
  diff.c: convert --[no-]abbrev
  diff.c: convert --[src|dst]-prefix
  diff.c: convert --line-prefix
  diff.c: convert --no-prefix
  diff.c: convert --inter-hunk-context
  diff.c: convert --color-moved
  diff.c: convert --color-moved-ws
  diff.c: allow --no-color-moved-ws
  range-diff: use parse_options() instead of diff_opt_parse()
  diff --no-index: use parse_options() instead of diff_opt_parse()
  am: avoid diff_opt_parse()

 Documentation/diff-options.txt |   24 +-
 builtin/am.c                   |    4 +-
 builtin/blame.c                |    2 +-
 builtin/diff.c                 |   21 +-
 builtin/merge.c                |    9 +-
 builtin/range-diff.c           |   26 +-
 builtin/update-index.c         |   41 +-
 diff-no-index.c                |   49 +-
 diff.c                         | 1136 ++++++++++++++++++++------------
 diff.h                         |   85 +--
 parse-options-cb.c             |   11 +-
 parse-options.c                |  152 +++--
 parse-options.h                |  114 ++--
 t/t4053-diff-no-index.sh       |    3 +-
 t/t7800-difftool.sh            |    4 +-
 15 files changed, 1054 insertions(+), 627 deletions(-)

Comments

Ævar Arnfjörð Bjarmason Jan. 17, 2019, 2:32 p.m. UTC | #1 
On Thu, Jan 17 2019, Nguyễn Thái Ngọc Duy wrote:

> This series converts diff option parsing to using parse-options. There
> are a couple benefits of using parse-options, including "git <cmd> -h"
> output, completion and less duplicate work.
>
> This is the first half. The second one would be converting the option
> parser in revision.c.  After that, the end game is, any command can
> take a 'struct option[]' somewhere from diff/rev code, remove the
> options they are not interested, then merge with their own options and
> do parse_options() just once. There will be no separate parse phase
> for revision/diff anymore.
>
> I sent a sneak peek [1] last year and got two good comments. I take it
> people at least did not oppose to this. The most interesting parts are
> at the top and bottom. The middle is just boring conversion, usually
> one option per patch.

I'm very much for this, and have skimmed it (but not stress tested) and
it looks good to me. I have some WIP patches to --abbrev that conflict,
but which will be simpler as a result of this.
Stefan Beller Jan. 17, 2019, 7:50 p.m. UTC | #2 
On Thu, Jan 17, 2019 at 6:33 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:

> > I sent a sneak peek [1] last year and got two good comments. I take it
> > people at least did not oppose to this. The most interesting parts are
> > at the top and bottom. The middle is just boring conversion, usually
> > one option per patch.
>
> I'm very much for this, and have skimmed it (but not stress tested) and
> it looks good to me. I have some WIP patches to --abbrev that conflict,
> but which will be simpler as a result of this.

I like the series a lot.