git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Daniel Ferreira <bnmvco@gmail.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, Johannes.Schindelin@gmx.de,
	Daniel Ferreira <bnmvco@gmail.com>
Subject: [PATCH 0/3] Port git-add--interactive.perl:status_cmd to C
Date: Fri,  5 May 2017 15:43:37 -0300	[thread overview]
Message-ID: <1494009820-2090-1-git-send-email-bnmvco@gmail.com> (raw)

Hm, it looks like my GSoC project won't be in the Git organization,
although I'm still interested in going for this so I guess I'll send
patches to implement my proposal anyway (although certainly in a slower
pace than I would if on the program).

This series introduces git-add-interactive--helper (or should it be
called git-add--interactive--helper?) as a builtin capable of doing
what the Perl script's status_cmd() would do.

I wish this patch series could have been smaller, although I don't
think it would make sense to bring add-interactive "subunits" smaller
than a command to the helper, and status_cmd (aside from probably
add_untracked_cmd) was the simplest one to do after all -- and still
required ~250 lines on the new builtin.

Another regret I had was not being able to retire any code from the
Perl script yet (and will likely not be able to do so until all
commands have been ported), but that is not such a big thing after
all.

As for the new builtin, I think the color handling code is pretty
straightforward. In fact, it was practically copied from places like
clean.c or diff.c (which makes me wonder if some of that code could
be generalized to avoid duplication). The same goes for the pretty
simple option parsing code.

Bigger issues seem to arise when dealing with getting the numstat.
While (as Junio anticipated on an RFC) some tasks like getting the
actual diff and splitting it may require making the "diff
machinery" write to a temporary file that we will read and do things
with, I think it would be weird to do that for parsing a simple
numstat from it. My first instinct was to create something like
show_numstat_interactive() or something on diff.c (analogous to the
other show_* functions). Doing that, however, would stumble upon
another issue: we would not be able to print both a file's diff
against the index (obtained from run_diff_index) and against the
worktree (obtained from run_diff_files) in that function. The solution
I came up with was to export the diffstat interface from diff.c into
the world and allow our new builtin to use that and build our status_cmd
output. Unless this breaks some rule of Git's API design, the result
seems pretty reasonable to me.

Travis CI build: https://travis-ci.org/theiostream/git/builds/229232202

Looking forward for feedback,
Daniel.

Daniel Ferreira (3):
  diff: export diffstat interface
  add--interactive: add builtin helper for interactive add
  add--interactive: use add-interactive--helper for status_cmd

 .gitignore                        |   1 +
 Makefile                          |   1 +
 builtin.h                         |   1 +
 builtin/add-interactive--helper.c | 258 ++++++++++++++++++++++++++++++++++++++
 diff.c                            |  17 +--
 diff.h                            |  19 ++-
 git-add--interactive.perl         |   4 +-
 git.c                             |   1 +
 8 files changed, 282 insertions(+), 20 deletions(-)
 create mode 100644 builtin/add-interactive--helper.c

--
2.7.4 (Apple Git-66)


             reply	other threads:[~2017-05-05 18:44 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-05 18:43 Daniel Ferreira [this message]
2017-05-05 18:43 ` [PATCH 1/3] diff: export diffstat interface Daniel Ferreira
2017-05-05 21:28   ` Johannes Schindelin
2017-05-05 18:43 ` [PATCH 2/3] add--interactive: add builtin helper for interactive add Daniel Ferreira
2017-05-05 20:16   ` Ævar Arnfjörð Bjarmason
2017-05-05 21:21     ` Johannes Schindelin
2017-05-05 22:09       ` Ævar Arnfjörð Bjarmason
2017-05-05 22:30   ` Johannes Schindelin
2017-05-05 22:49     ` Ævar Arnfjörð Bjarmason
2017-05-08 11:35       ` Johannes Schindelin
2017-05-05 23:13     ` Daniel Ferreira (theiostream)
2017-05-05 23:28       ` Ævar Arnfjörð Bjarmason
2017-05-08 12:15       ` Johannes Schindelin
2017-05-05 18:43 ` [PATCH 3/3] add--interactive: use add-interactive--helper for status_cmd Daniel Ferreira
2017-05-05 22:32   ` Johannes Schindelin
2017-05-05 19:31 ` [PATCH 0/3] Port git-add--interactive.perl:status_cmd to C Ævar Arnfjörð Bjarmason
2017-05-05 22:33   ` Johannes Schindelin
2017-05-05 22:35   ` Jonathan Nieder
2017-05-05 22:38 ` Johannes Schindelin
2017-05-05 23:37   ` Daniel Ferreira (theiostream)
2017-05-08 12:23     ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1494009820-2090-1-git-send-email-bnmvco@gmail.com \
    --to=bnmvco@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).