* Re: [PATCH] commit: detect commits that exist in commit-graph but not in the ODB
From: Junio C Hamano @ 2023-10-20 17:35 UTC (permalink / raw)
To: Jeff King; +Cc: Patrick Steinhardt, git, Karthik Nayak, Taylor Blau
In-Reply-To: <20231020100024.GA2194074@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I guess that is really like a global variable but even more gross. ;)
Yeah, I tend to agree, but ...
> But it is nice that it can cross process boundaries, because "how
> careful do we want to be" may be in the eye of the caller, especially
> for plumbing commands. E.g., even without --missing, you may want
> "rev-list" to be extra careful about checking the existence of commits.
... these are all very good points.
> The caller in check_connected() could arguably turn that on by default
> (the way we used to turn on GIT_REF_PARANOIA for pruning repacks before
> it became the default).
True.
Thanks.
^ permalink raw reply
* Re: [PATCH v3 3/3] rev-list: add commit object support in `--missing` option
From: Junio C Hamano @ 2023-10-20 17:45 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git, ps
In-Reply-To: <CAOLa=ZT6qEsVfNETYua=RjjepDsXFj8uSdphS2LR1gGK1_sGFg@mail.gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
> Was trying to use bit number 12, which coincides METAINFO_SHOWN in
> builtin/blame.c.
> From skimming over the code, METAINFO_SHOWN is used only within
> blame.c and there
> should not be collisions here since blame.c doesn't set the
> do_not_die_on_missing_objects bit
> either.
Thanks for researching. It sounds like it may be a better bit to
steal than the one used by the commit-graph, as long as there is no
reason to expect that blame may want to work in a corrupt repository
with missing objects, but when it happens, we may regret the
decision we are making here.
Thanks.
^ permalink raw reply
* Re: [PATCH v2] builtin/branch.c: adjust error messages to coding guidelines
From: Isoken Ibizugbe @ 2023-10-20 17:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Rubén Justo, git, christian.couder
In-Reply-To: <xmqqwmvhqjyx.fsf@gitster.g>
On Fri, Oct 20, 2023 at 6:31 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Rubén Justo <rjusto@gmail.com> writes:
>
> > On 19-oct-2023 09:40:51, Isoken June Ibizugbe wrote:
> >
> >> As per the CodingGuidelines document, it is recommended that a single-line
> >> message provided to error messages such as die(), error() and warning(),
> >
> > This is confusing; some multi-line messages are fixed in this series.
> >
> >> should start with a lowercase letter and should not end with a period.
> >> Also this patch fixes the tests broken by the changes.
>
> "Also this patch fixes the tests broken by the changes" -> "Adjust
> tests to match updated messages".
>
> > Well done, describing why the tests are touched.
>
> Nicely reviewed. But it is unclear to me how we want to mention
> updates to multi-line messages. Do we have a good reference in the
> guidelines we can copy?
>
> The general desire is for a single-liner to look like so:
>
> error: the branch 'foo' is not fully merged
>
> and an untold assumption is that we strongly prefer a single
> sentence in a single-liner error message---a full-stop to separate
> multiple sentences can be omitted for brevity.
>
> But we have follow-up sentences that are not strictly "errors" in
> some messages, e.g.,
>
> >> - error(_("The branch '%s' is not fully merged.\n"
> >> + error(_("the branch '%s' is not fully merged.\n"
> >> "If you are sure you want to delete it, "
> >> - "run 'git branch -D %s'."), branchname, branchname);
> >> + "run 'git branch -D %s'"), branchname, branchname);
>
> In a modern codebase with facilities from advice.c, we would
> probably do "a single-liner error message, optionally followed by a
> hint message", i.e.
>
> error: the branch 'foo' is not fully merged
> hint: If you are sure you want to delete it,
> hint: run "git branch -D foo".
>
> but this message apparently predates wide use of the advice_if() and
> friends.
>
> But rewriting this error message to use advice is probably outside
> the scope of this patch. But for completeness it would probably
> look something like this (with necessary ADVICE_FOO defined):
>
> error(_("the branch '%s' is not fully merged"), branchname);
> advice_if_enabled(ADVICE_DELETE_BRANCH,
> _("If you are sure you want to delete it,"
> "run 'git branch -D %s'."));
>
> If I were doing this patch, I'd probably just add
>
> /* NEEDSWORK: turn the remainder ofthe message into advice */
>
Thanks for the feedback.
Are suggesting I add this comment to error messages that are of this nature?
> in front of the existing error() call for this one without touching
> the message itself, as it will have to be touched again when such a
> change happens.
>
> Thanks.
^ permalink raw reply
* [PATCH v3] grep: die gracefully when outside repository
From: Kristoffer Haugsbakk @ 2023-10-20 18:04 UTC (permalink / raw)
To: code; +Cc: gitster, sunshine, ks1322, git
In-Reply-To: <087c92e3904dd774f672373727c300bf7f5f6369.1697317276.git.code@khaugsbakk.name>
Die gracefully when `git grep --no-index` is run outside of a Git
repository and the path is outside the directory tree.
If you are not in a Git repository and say:
git grep --no-index search ..
You trigger a `BUG`:
BUG: environment.c:213: git environment hasn't been setup
Aborted (core dumped)
Because `..` is a valid path which is treated as a pathspec. Then
`pathspec` figures out that it is not in the current directory tree. The
`BUG` is triggered when `pathspec` tries to advise the user about how the
path is not in the current (non-existing) repository.
Reported-by: ks1322 ks1322 <ks1322@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
Notes (series):
v2:
- Initialize `hint_path` after we know that we are in a Git repository
- Apply Junio's suggestion for the test: https://lore.kernel.org/git/xmqqzg0hf0g8.fsf@gitster.g/
v3:
- commit message: correct to “advise” (“advice” is a noun)
pathspec.c | 7 ++++++-
t/t7810-grep.sh | 27 +++++++++++++++++++++++++++
2 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/pathspec.c b/pathspec.c
index 3a3a5724c44..264b4929a55 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -467,7 +467,12 @@ static void init_pathspec_item(struct pathspec_item *item, unsigned flags,
match = prefix_path_gently(prefix, prefixlen,
&prefixlen, copyfrom);
if (!match) {
- const char *hint_path = get_git_work_tree();
+ const char *hint_path;
+
+ if (!have_git_dir())
+ die(_("'%s' is outside the directory tree"),
+ copyfrom);
+ hint_path = get_git_work_tree();
if (!hint_path)
hint_path = get_git_dir();
die(_("%s: '%s' is outside repository at '%s'"), elt,
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 39d6d713ecb..84838c0fe1b 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -1234,6 +1234,33 @@ test_expect_success 'outside of git repository with fallbackToNoIndex' '
)
'
+test_expect_success 'no repository with path outside $cwd' '
+ test_when_finished rm -fr non &&
+ rm -fr non &&
+ mkdir -p non/git/sub non/tig &&
+ (
+ GIT_CEILING_DIRECTORIES="$(pwd)/non" &&
+ export GIT_CEILING_DIRECTORIES &&
+ cd non/git &&
+ test_expect_code 128 git grep --no-index search .. 2>error &&
+ grep "is outside the directory tree" error
+ ) &&
+ (
+ GIT_CEILING_DIRECTORIES="$(pwd)/non" &&
+ export GIT_CEILING_DIRECTORIES &&
+ cd non/git &&
+ test_expect_code 128 git grep --no-index search ../tig 2>error &&
+ grep "is outside the directory tree" error
+ ) &&
+ (
+ GIT_CEILING_DIRECTORIES="$(pwd)/non" &&
+ export GIT_CEILING_DIRECTORIES &&
+ cd non/git &&
+ test_expect_code 128 git grep --no-index search ../non 2>error &&
+ grep "no such path in the working tree" error
+ )
+'
+
test_expect_success 'inside git repository but with --no-index' '
rm -fr is &&
mkdir -p is/git/sub &&
--
2.42.0.2.g879ad04204
^ permalink raw reply related
* Re: [PATCH v2] grep: die gracefully when outside repository
From: Junio C Hamano @ 2023-10-20 18:06 UTC (permalink / raw)
To: Eric Sunshine; +Cc: Kristoffer Haugsbakk, ks1322, git
In-Reply-To: <CAPig+cTBYw9=Wo=TR8MD5xX9hgurnfR2Xzc_wHSYnL1R00=xpw@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
> On Fri, Oct 20, 2023 at 12:40 PM Kristoffer Haugsbakk
> <code@khaugsbakk.name> wrote:
>> Die gracefully when `git grep --no-index` is run outside of a Git
>> repository and the path is outside the directory tree.
>>
>> If you are not in a Git repository and say:
>>
>> git grep --no-index search ..
>>
>> You trigger a `BUG`:
>>
>> BUG: environment.c:213: git environment hasn't been setup
>> Aborted (core dumped)
>>
>> Because `..` is a valid path which is treated as a pathspec. Then
>> `pathspec` figures out that it is not in the current directory tree. The
>> `BUG` is triggered when `pathspec` tries to advice the user about how the
>> path is not in the current (non-existing) repository.
>
> s/advice/advise/
>
> (probably not worth a reroll)
The only remaining niggle I have is that the effect of this change
would be much wider than just "grep", but in "git shortlog" output
it may appear that this is specific to it, making later developers'
life a bit harder when they are hunting for the cause of a behaviour
change that is outside "grep", but still caused by this patch.
But I think I am worried too much in this particular case. Once
this codepath is entered, the code will die no matter what, and we
are merely making it die a bit more nicely.
There still is the "we say different things depending on the path
outside the hierarchy exists or not" raised by Peff remaining, but
for now, let's declare a victory and merge it to 'next'.
Thanks.
^ permalink raw reply
* [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Jacob Stopak @ 2023-10-20 18:39 UTC (permalink / raw)
To: git; +Cc: Jacob Stopak
This is a proposal / proof-of-concept for a new table-based output
format for the git status command, and for dry runs (-n) of the git add
command. This could be extended to create visual dry runs for other
other commands like rm, mv, restore, stash, commit, and clean.
For some context, earlier this year I released a tool called Git-Sim
(https://github.com/initialcommit-com/git-sim) which allows users to do
visual dry runs of many Git commands, which are rendered as high quality
output image files. Simulating commands like status, add, rm, mv, restore,
stash, and commit creates a table with 3 columns to represent the way file
changes "move around" as a result of the command being simulated.
I've gotten positive feedback from users about this visual approach to
simulating git commands, which is more intuitive than pure terminal text
for both newer users to understand how git works and for visual people.
As a result, I was thinking of ways to integrate these types of visual
formats directly into Git. A table-based output format with colored
highlighting for the commands mentioned above is low hanging fruit.
Teach 'git status' the new -t, --table flag, which displays the status
output in a 3-column table format, preserving terminology and color
coding from the default git status "long output" format (note that the
column headers are shortened here for the small width of this email, and
also I just realized that the tables below might not look right on the
mailing list due to the differing character width, but it looks correct
in the terminal so please test there it's more fun anyway :D):
$ git status -t
-------------------------------------------------------------------------
| Untracked files | Changes n...or commit | Changes t...committed |
-------------------------------------------------------------------------
| poiu | | |
| status-table/ | | |
| | | asdf |
| | table.c | |
| | wt-status.c | |
-------------------------------------------------------------------------
Teach 'git add' the new -t, --table flag to be used ONLY in combination
with the '-n' flag for dry runs. Instead of simply printing out the
added filenames, the full status table format is displayed, along with
arrows that visually show how the added files are being moved around:
$ git add -nt poiu wt-status.c
-------------------------------------------------------------------------
| Untracked files | Changes n...or commit | Changes t...committed |
-------------------------------------------------------------------------
| poiu -----------------------------------------> poiu |
| status-table/ | | |
| | | asdf |
| | table.c | |
| | wt-status.c ----------> wt-status.c |
-------------------------------------------------------------------------
Other notes:
* The width of the table and columns are dynamically set based on the
width of the terminal.
* Long paths are shortened to include the maximum number of characters
from both ends of the path that will fit, with a '...' in the middle.
* Color coding matches the default output of 'git status', with
untracked files and working dir mods in red, and staged changes in
green. If needed, arrows are drawn in cyan.
As stated above, the dry run version of the table format can be applied
to various other commands like rm, mv, restore, stash, commit, and clean
which all move file changes around in a way that can be represented in
the table format. New columns may need to be added or arrows reversed
to show changes moving in various directions. Note that some of these
commands don't appear to have a dry run (-n) option yet, so it could be
added for consistency (if not already in use) and for use with the new
table format.
Since this is an RFC patch series, I probably did some illegal and dumb
things in my code changes just to get it into a demo-able state. I am a
bit wary of having made changes to files like "read-cache.c" and
"read-cache-ll.h" to pass in the wt_status info, and there are probably
betters ways to do some other things too.
Feedback on both the new format itself and the implementation is very
much appreciated!
Jacob Stopak (5):
status: introduce -t, --table flag
status: handle long paths with -t, --table flag
status: add advice arg for -t, --table flag
add: add -t, --table flag for visual dry runs
add: set unique color for -t, --table arrows
Makefile | 1 +
builtin/add.c | 46 +++++++--
builtin/commit.c | 4 +-
read-cache-ll.h | 9 +-
read-cache.c | 32 ++++++-
table.c | 245 +++++++++++++++++++++++++++++++++++++++++++++++
table.h | 6 ++
wt-status.c | 74 +++++++++-----
wt-status.h | 3 +
9 files changed, 378 insertions(+), 42 deletions(-)
create mode 100644 table.c
create mode 100644 table.h
--
2.42.0.402.gbe8243af7b.dirty
^ permalink raw reply
* [RFC PATCH 1/5] status: introduce -t, --table flag
From: Jacob Stopak @ 2023-10-20 18:39 UTC (permalink / raw)
To: git; +Cc: Jacob Stopak
In-Reply-To: <20231020183947.463882-1-jacob@initialcommit.io>
Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
---
Makefile | 1 +
builtin/commit.c | 2 +
table.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++
table.h | 6 +++
wt-status.c | 72 +++++++++++++++++++----------
wt-status.h | 1 +
6 files changed, 174 insertions(+), 25 deletions(-)
create mode 100644 table.c
create mode 100644 table.h
diff --git a/Makefile b/Makefile
index 9c6a2f125f..a7399ca8f0 100644
--- a/Makefile
+++ b/Makefile
@@ -1155,6 +1155,7 @@ LIB_OBJS += submodule-config.o
LIB_OBJS += submodule.o
LIB_OBJS += symlinks.o
LIB_OBJS += tag.o
+LIB_OBJS += table.o
LIB_OBJS += tempfile.o
LIB_OBJS += thread-utils.o
LIB_OBJS += tmp-objdir.o
diff --git a/builtin/commit.c b/builtin/commit.c
index 7da5f92448..4338896dbf 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1539,6 +1539,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)
OPT_CALLBACK_F('M', "find-renames", &rename_score_arg,
N_("n"), N_("detect renames, optionally set similarity index"),
PARSE_OPT_OPTARG | PARSE_OPT_NONEG, opt_parse_rename_score),
+ OPT_SET_INT('t', "table", &status_format,
+ N_("show status in table format"), STATUS_FORMAT_TABLE),
OPT_END(),
};
diff --git a/table.c b/table.c
new file mode 100644
index 0000000000..54cf9e4d07
--- /dev/null
+++ b/table.c
@@ -0,0 +1,117 @@
+#define USE_THE_INDEX_VARIABLE
+#include "builtin.h"
+#include "gettext.h"
+#include "strbuf.h"
+#include "wt-status.h"
+#include "config.h"
+#include "string-list.h"
+#include "sys/ioctl.h"
+
+static const char *color(int slot, struct wt_status *s)
+{
+ const char *c = "";
+ if (want_color(s->use_color))
+ c = s->color_palette[slot];
+ if (slot == WT_STATUS_ONBRANCH && color_is_nil(c))
+ c = s->color_palette[WT_STATUS_HEADER];
+ return c;
+}
+
+static void build_table_border(struct strbuf *buf, int cols)
+{
+ strbuf_reset(buf);
+ strbuf_addchars(buf, '-', cols);
+}
+
+static void build_table_entry(struct strbuf *buf, char *entry, int cols)
+{
+ strbuf_reset(buf);
+ strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2);
+ strbuf_addstr(buf, entry);
+
+ /* Bump right padding if entry length is odd */
+ if (!(strlen(entry) % 2))
+ strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2 + 1);
+ else
+ strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2);
+}
+
+static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
+{
+ printf(_("|"));
+ color_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), "%s", buf1->buf);
+ printf(_("|"));
+ color_fprintf(s->fp, color(WT_STATUS_CHANGED, s), "%s", buf2->buf);
+ printf(_("|"));
+ color_fprintf(s->fp, color(WT_STATUS_UPDATED, s), "%s", buf3->buf);
+ printf(_("|\n"));
+}
+
+void build_and_draw_status_table(struct wt_status *s)
+{
+ struct winsize w;
+ int cols;
+ struct strbuf table_border = STRBUF_INIT;
+ struct strbuf table_col_entry_1 = STRBUF_INIT;
+ struct strbuf table_col_entry_2 = STRBUF_INIT;
+ struct strbuf table_col_entry_3 = STRBUF_INIT;
+ struct string_list_item *item;
+
+ /* Get terminal width */
+ ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
+ cols = w.ws_col;
+
+ /* Ensure table is divisible into 3 even columns */
+ while (((cols - 1) % 3) > 0 || !(cols % 2)) {
+ cols -= 1;
+ }
+
+ build_table_border(&table_border, cols);
+ build_table_entry(&table_col_entry_1, "Untracked files", cols);
+ build_table_entry(&table_col_entry_2, "Changes not staged for commit", cols);
+ build_table_entry(&table_col_entry_3, "Changes to be committed", cols);
+
+ /* Draw table header */
+ printf(_("%s\n"), table_border.buf);
+ printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
+ printf(_("%s\n"), table_border.buf);
+
+ /* Draw table body */
+ for_each_string_list_item(item, &s->untracked) {
+ build_table_entry(&table_col_entry_1, item->string, cols);
+ build_table_entry(&table_col_entry_2, "", cols);
+ build_table_entry(&table_col_entry_3, "", cols);
+ print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+ }
+
+ for_each_string_list_item(item, &s->change) {
+ struct wt_status_change_data *d = item->util;
+ if (d->worktree_status && d->index_status) {
+ build_table_entry(&table_col_entry_1, "", cols);
+ build_table_entry(&table_col_entry_2, item->string, cols);
+ build_table_entry(&table_col_entry_3, item->string, cols);
+ } else if (d->worktree_status) {
+ build_table_entry(&table_col_entry_1, "", cols);
+ build_table_entry(&table_col_entry_2, item->string, cols);
+ build_table_entry(&table_col_entry_3, "", cols);
+ } else if (d->index_status) {
+ build_table_entry(&table_col_entry_1, "", cols);
+ build_table_entry(&table_col_entry_2, "", cols);
+ build_table_entry(&table_col_entry_3, item->string, cols);
+ }
+ print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+ }
+
+ if (!s->untracked.nr && !s->change.nr) {
+ build_table_entry(&table_col_entry_1, "-", cols);
+ build_table_entry(&table_col_entry_2, "-", cols);
+ build_table_entry(&table_col_entry_3, "-", cols);
+ printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
+ }
+
+ printf(_("%s\n"), table_border.buf);
+ strbuf_release(&table_border);
+ strbuf_release(&table_col_entry_1);
+ strbuf_release(&table_col_entry_2);
+ strbuf_release(&table_col_entry_3);
+}
diff --git a/table.h b/table.h
new file mode 100644
index 0000000000..30e0d5509b
--- /dev/null
+++ b/table.h
@@ -0,0 +1,6 @@
+#ifndef TABLE_H
+#define TABLE_H
+
+void build_and_draw_status_table(struct wt_status *s);
+
+#endif /* TABLE_H */
diff --git a/wt-status.c b/wt-status.c
index 9f45bf6949..24b56ea559 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -31,6 +31,7 @@
#include "lockfile.h"
#include "sequencer.h"
#include "fsmonitor-settings.h"
+#include "table.h"
#define AB_DELAY_WARNING_IN_MS (2 * 1000)
#define UF_DELAY_WARNING_IN_MS (2 * 1000)
@@ -1833,39 +1834,46 @@ static void wt_longstatus_print_state(struct wt_status *s)
show_sparse_checkout_in_use(s, state_color);
}
-static void wt_longstatus_print(struct wt_status *s)
+static void wt_longstatus_print_onwhat(struct wt_status *s, const char *branch_name)
{
+ const char *on_what = _("On branch ");
const char *branch_color = color(WT_STATUS_ONBRANCH, s);
const char *branch_status_color = color(WT_STATUS_HEADER, s);
+
+ if (!strcmp(branch_name, "HEAD")) {
+ branch_status_color = color(WT_STATUS_NOBRANCH, s);
+ if (s->state.rebase_in_progress ||
+ s->state.rebase_interactive_in_progress) {
+ if (s->state.rebase_interactive_in_progress)
+ on_what = _("interactive rebase in progress; onto ");
+ else
+ on_what = _("rebase in progress; onto ");
+ branch_name = s->state.onto;
+ } else if (s->state.detached_from) {
+ branch_name = s->state.detached_from;
+ if (s->state.detached_at)
+ on_what = _("HEAD detached at ");
+ else
+ on_what = _("HEAD detached from ");
+ } else {
+ branch_name = "";
+ on_what = _("Not currently on any branch.");
+ }
+ } else
+ skip_prefix(branch_name, "refs/heads/", &branch_name);
+
+ status_printf_more(s, branch_status_color, "%s", on_what);
+ status_printf_more(s, branch_color, "%s\n", branch_name);
+}
+
+static void wt_longstatus_print(struct wt_status *s)
+{
enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(s->repo);
if (s->branch) {
- const char *on_what = _("On branch ");
const char *branch_name = s->branch;
- if (!strcmp(branch_name, "HEAD")) {
- branch_status_color = color(WT_STATUS_NOBRANCH, s);
- if (s->state.rebase_in_progress ||
- s->state.rebase_interactive_in_progress) {
- if (s->state.rebase_interactive_in_progress)
- on_what = _("interactive rebase in progress; onto ");
- else
- on_what = _("rebase in progress; onto ");
- branch_name = s->state.onto;
- } else if (s->state.detached_from) {
- branch_name = s->state.detached_from;
- if (s->state.detached_at)
- on_what = _("HEAD detached at ");
- else
- on_what = _("HEAD detached from ");
- } else {
- branch_name = "";
- on_what = _("Not currently on any branch.");
- }
- } else
- skip_prefix(branch_name, "refs/heads/", &branch_name);
status_printf(s, color(WT_STATUS_HEADER, s), "%s", "");
- status_printf_more(s, branch_status_color, "%s", on_what);
- status_printf_more(s, branch_color, "%s\n", branch_name);
+ wt_longstatus_print_onwhat(s, branch_name);
if (!s->is_initial)
wt_longstatus_print_tracking(s);
}
@@ -2133,6 +2141,17 @@ static void wt_shortstatus_print(struct wt_status *s)
wt_shortstatus_other(it, s, "!!");
}
+static void wt_tablestatus_print(struct wt_status *s)
+{
+ if (s->show_branch) {
+ const char *branch_name = s->branch;
+ wt_longstatus_print_onwhat(s, branch_name);
+ wt_longstatus_print_tracking(s);
+ }
+
+ build_and_draw_status_table(s);
+}
+
static void wt_porcelain_print(struct wt_status *s)
{
s->use_color = 0;
@@ -2560,6 +2579,9 @@ void wt_status_print(struct wt_status *s)
case STATUS_FORMAT_LONG:
wt_longstatus_print(s);
break;
+ case STATUS_FORMAT_TABLE:
+ wt_tablestatus_print(s);
+ break;
}
trace2_region_leave("status", "print", s->repo);
diff --git a/wt-status.h b/wt-status.h
index ab9cc9d8f0..70a3b7a2e4 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -73,6 +73,7 @@ enum wt_status_format {
STATUS_FORMAT_SHORT,
STATUS_FORMAT_PORCELAIN,
STATUS_FORMAT_PORCELAIN_V2,
+ STATUS_FORMAT_TABLE,
STATUS_FORMAT_UNSPECIFIED
};
--
2.42.0.402.gbe8243af7b.dirty
^ permalink raw reply related
* [RFC PATCH 2/5] status: handle long paths with -t, --table flag
From: Jacob Stopak @ 2023-10-20 18:39 UTC (permalink / raw)
To: git; +Cc: Jacob Stopak
In-Reply-To: <20231020183947.463882-1-jacob@initialcommit.io>
Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
---
table.c | 39 ++++++++++++++++++++++++++++++++++-----
1 file changed, 34 insertions(+), 5 deletions(-)
diff --git a/table.c b/table.c
index 54cf9e4d07..87b6df8c66 100644
--- a/table.c
+++ b/table.c
@@ -25,15 +25,44 @@ static void build_table_border(struct strbuf *buf, int cols)
static void build_table_entry(struct strbuf *buf, char *entry, int cols)
{
+ int len = strlen(entry);
+ size_t col_width = (cols / 3) - 5; /* subtract for padding */
+
strbuf_reset(buf);
- strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2);
+
+ /* Trim equally from both sides if it doesn't fit in column */
+ if (len > col_width) {
+ struct strbuf start_buf = STRBUF_INIT;
+ struct strbuf end_buf = STRBUF_INIT;
+ struct strbuf entry_buf = STRBUF_INIT;
+
+ strbuf_addstr(&start_buf, entry);
+ strbuf_addstr(&end_buf, entry);
+
+ strbuf_remove(&start_buf, col_width / 2, len - col_width / 2);
+ strbuf_remove(&end_buf, 0, len - col_width / 2);
+
+ strbuf_addstr(&entry_buf, start_buf.buf);
+ strbuf_addstr(&entry_buf, "...");
+ strbuf_addstr(&entry_buf, end_buf.buf);
+
+ entry = strbuf_detach(&entry_buf, &col_width);
+ len = strlen(entry);
+
+ strbuf_release(&start_buf);
+ strbuf_release(&end_buf);
+ strbuf_release(&entry_buf);
+ }
+
+ strbuf_addchars(buf, ' ', (cols / 3 - len - 1) / 2); /* left padding */
strbuf_addstr(buf, entry);
- /* Bump right padding if entry length is odd */
- if (!(strlen(entry) % 2))
- strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2 + 1);
+ /* right padding */
+ if (!(len % 2))
+ /* Bump right padding if entry length is odd */
+ strbuf_addchars(buf, ' ', (cols / 3 - len - 1) / 2 + 1);
else
- strbuf_addchars(buf, ' ', (cols / 3 - 1 - strlen(entry)) / 2);
+ strbuf_addchars(buf, ' ', (cols / 3 - len - 1) / 2);
}
static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
--
2.42.0.402.gbe8243af7b.dirty
^ permalink raw reply related
* [RFC PATCH 3/5] status: add advice arg for -t, --table flag
From: Jacob Stopak @ 2023-10-20 18:39 UTC (permalink / raw)
To: git; +Cc: Jacob Stopak
In-Reply-To: <20231020183947.463882-1-jacob@initialcommit.io>
Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
---
table.c | 19 +++++++++++++++++--
table.h | 2 +-
wt-status.c | 2 +-
3 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/table.c b/table.c
index 87b6df8c66..73751339da 100644
--- a/table.c
+++ b/table.c
@@ -76,7 +76,7 @@ static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, stru
printf(_("|\n"));
}
-void build_and_draw_status_table(struct wt_status *s)
+void build_and_draw_status_table(struct wt_status *s, int add_advice)
{
struct winsize w;
int cols;
@@ -95,14 +95,29 @@ void build_and_draw_status_table(struct wt_status *s)
cols -= 1;
}
+ /* Draw table header */
build_table_border(&table_border, cols);
build_table_entry(&table_col_entry_1, "Untracked files", cols);
build_table_entry(&table_col_entry_2, "Changes not staged for commit", cols);
build_table_entry(&table_col_entry_3, "Changes to be committed", cols);
- /* Draw table header */
printf(_("%s\n"), table_border.buf);
printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
+
+ if (add_advice) {
+ build_table_entry(&table_col_entry_1, "(stage: git add <file>)", cols);
+ build_table_entry(&table_col_entry_2, "(stage: git add <file>)", cols);
+ build_table_entry(&table_col_entry_3, "(unstage: git restore --staged <file>)", cols);
+
+ printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
+
+ build_table_entry(&table_col_entry_1, "", cols);
+ build_table_entry(&table_col_entry_2, "(discard: git restore --staged <file>)", cols);
+ build_table_entry(&table_col_entry_3, "", cols);
+
+ printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
+ }
+
printf(_("%s\n"), table_border.buf);
/* Draw table body */
diff --git a/table.h b/table.h
index 30e0d5509b..6017923bf9 100644
--- a/table.h
+++ b/table.h
@@ -1,6 +1,6 @@
#ifndef TABLE_H
#define TABLE_H
-void build_and_draw_status_table(struct wt_status *s);
+void build_and_draw_status_table(struct wt_status *s, int i);
#endif /* TABLE_H */
diff --git a/wt-status.c b/wt-status.c
index 24b56ea559..62731859fe 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -2149,7 +2149,7 @@ static void wt_tablestatus_print(struct wt_status *s)
wt_longstatus_print_tracking(s);
}
- build_and_draw_status_table(s);
+ build_and_draw_status_table(s, 0);
}
static void wt_porcelain_print(struct wt_status *s)
--
2.42.0.402.gbe8243af7b.dirty
^ permalink raw reply related
* [RFC PATCH 5/5] add: set unique color for -t, --table arrows
From: Jacob Stopak @ 2023-10-20 18:39 UTC (permalink / raw)
To: git; +Cc: Jacob Stopak
In-Reply-To: <20231020183947.463882-1-jacob@initialcommit.io>
Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
---
table.c | 62 +++++++++++++++++++++++++++++++----------------------
wt-status.c | 1 +
wt-status.h | 1 +
3 files changed, 38 insertions(+), 26 deletions(-)
diff --git a/table.c b/table.c
index a6fc660fec..390b2e2dd9 100644
--- a/table.c
+++ b/table.c
@@ -5,6 +5,7 @@
#include "wt-status.h"
#include "config.h"
#include "string-list.h"
+#include "color.h"
#include "sys/ioctl.h"
static const char *color(int slot, struct wt_status *s)
@@ -65,52 +66,51 @@ static void build_table_entry(struct strbuf *buf, char *entry, int cols)
strbuf_addchars(buf, ' ', (cols / 3 - len - 1) / 2);
}
-static void add_arrow_to_entry(struct strbuf *buf, int add_after_entry)
+static void build_arrow(struct strbuf *buf, struct strbuf* arrow, int add_after_entry)
{
struct strbuf empty = STRBUF_INIT;
struct strbuf trimmed = STRBUF_INIT;
- struct strbuf holder = STRBUF_INIT;
int len = strlen(buf->buf);
+ strbuf_reset(arrow);
strbuf_addstr(&trimmed, buf->buf);
strbuf_trim(&trimmed);
if (!strbuf_cmp(&trimmed, &empty) && !add_after_entry) {
strbuf_reset(buf);
- strbuf_addchars(buf, '-', len + 1);
+ strbuf_addchars(arrow, '-', len + 1);
} else if (add_after_entry) {
strbuf_rtrim(buf);
- strbuf_addchars(buf, ' ', 1);
- strbuf_addchars(buf, '-', len - strlen(buf->buf) + 1);
+ strbuf_addchars(arrow, ' ', 1);
+ strbuf_addchars(arrow, '-', len - strlen(buf->buf) + 1);
} else if (!add_after_entry) {
strbuf_ltrim(buf);
- strbuf_addchars(&holder, '-', len - strlen(buf->buf) - 2);
- strbuf_addchars(&holder, '>', 1);
- strbuf_addchars(&holder, ' ', 1);
- strbuf_addstr(&holder, buf->buf);
- strbuf_reset(buf);
- strbuf_addstr(buf, holder.buf);
+ strbuf_addchars(arrow, '-', len - strlen(buf->buf) - 3);
+ strbuf_addchars(arrow, '>', 1);
+ strbuf_addchars(arrow, ' ', 1);
}
}
-static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s, int hide_pipe)
+static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct strbuf *arrow1, struct strbuf *arrow2, struct strbuf *arrow3, struct wt_status *s, int hide_pipe)
{
printf(_("|"));
color_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), "%s", buf1->buf);
+ if (strlen(arrow1->buf) > 0)
+ color_fprintf(s->fp, color(WT_STATUS_ARROW, s), "%s", arrow1->buf);
if (hide_pipe != 1 && hide_pipe != 3)
printf(_("|"));
color_fprintf(s->fp, color(WT_STATUS_CHANGED, s), "%s", buf2->buf);
+ if (strlen(arrow2->buf) > 0)
+ color_fprintf(s->fp, color(WT_STATUS_ARROW, s), "%s", arrow2->buf);
if (hide_pipe != 2 && hide_pipe != 3)
printf(_("|"));
+ if (strlen(arrow3->buf) > 0) {
+ color_fprintf(s->fp, color(WT_STATUS_ARROW, s), "%s", arrow3->buf);
+ }
color_fprintf(s->fp, color(WT_STATUS_UPDATED, s), "%s", buf3->buf);
printf(_("|\n"));
}
-static void print_table_body_line_(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
-{
- print_table_body_line(buf1, buf2, buf3, s, 0);
-}
-
void build_and_draw_status_table(struct wt_status *s, int advice)
{
struct winsize w;
@@ -119,6 +119,9 @@ void build_and_draw_status_table(struct wt_status *s, int advice)
struct strbuf table_col_entry_1 = STRBUF_INIT;
struct strbuf table_col_entry_2 = STRBUF_INIT;
struct strbuf table_col_entry_3 = STRBUF_INIT;
+ struct strbuf arrow_1 = STRBUF_INIT;
+ struct strbuf arrow_2 = STRBUF_INIT;
+ struct strbuf arrow_3 = STRBUF_INIT;
struct string_list_item *item, *item2;
/* Get terminal width */
@@ -170,17 +173,21 @@ void build_and_draw_status_table(struct wt_status *s, int advice)
strbuf_addstr(&buf_2, item2->string);
if (!strbuf_cmp(&buf_1, &buf_2)) {
build_table_entry(&table_col_entry_3, buf_1.buf, cols);
- add_arrow_to_entry(&table_col_entry_1, 1);
- add_arrow_to_entry(&table_col_entry_2, 0);
- add_arrow_to_entry(&table_col_entry_3, 0);
+ build_arrow(&table_col_entry_1, &arrow_1, 1);
+ build_arrow(&table_col_entry_2, &arrow_2, 0);
+ build_arrow(&table_col_entry_3, &arrow_3, 0);
is_arrow = 1;
}
}
if (!is_arrow)
- print_table_body_line_(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+ print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, &arrow_1, &arrow_2, &arrow_3, s, 0);
else
- print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s, 3);
+ print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, &arrow_1, &arrow_2, &arrow_3, s, 3);
+
+ strbuf_reset(&arrow_1);
+ strbuf_reset(&arrow_2);
+ strbuf_reset(&arrow_3);
}
for_each_string_list_item(item, &s->change) {
@@ -203,8 +210,8 @@ void build_and_draw_status_table(struct wt_status *s, int advice)
strbuf_addstr(&buf_2, item2->string);
if (!strbuf_cmp(&buf_1, &buf_2)) {
build_table_entry(&table_col_entry_3, buf_1.buf, cols);
- add_arrow_to_entry(&table_col_entry_2, 1);
- add_arrow_to_entry(&table_col_entry_3, 0);
+ build_arrow(&table_col_entry_2, &arrow_2, 1);
+ build_arrow(&table_col_entry_3, &arrow_3, 0);
is_arrow = 1;
}
}
@@ -215,9 +222,12 @@ void build_and_draw_status_table(struct wt_status *s, int advice)
}
if (!is_arrow)
- print_table_body_line_(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+ print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, &arrow_1, &arrow_2, &arrow_3, s, 0);
else
- print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s, 2);
+ print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, &arrow_1, &arrow_2, &arrow_3, s, 2);
+ strbuf_reset(&arrow_1);
+ strbuf_reset(&arrow_2);
+ strbuf_reset(&arrow_3);
}
if (!s->untracked.nr && !s->change.nr) {
diff --git a/wt-status.c b/wt-status.c
index 975cfc01a5..fe38260baa 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -49,6 +49,7 @@ static char default_wt_status_colors[][COLOR_MAXLEN] = {
GIT_COLOR_GREEN, /* WT_STATUS_LOCAL_BRANCH */
GIT_COLOR_RED, /* WT_STATUS_REMOTE_BRANCH */
GIT_COLOR_NIL, /* WT_STATUS_ONBRANCH */
+ GIT_COLOR_CYAN, /* WT_STATUS_ARROW */
};
static const char *color(int slot, struct wt_status *s)
diff --git a/wt-status.h b/wt-status.h
index 5d29c058c1..0517f81e1b 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -19,6 +19,7 @@ enum color_wt_status {
WT_STATUS_LOCAL_BRANCH,
WT_STATUS_REMOTE_BRANCH,
WT_STATUS_ONBRANCH,
+ WT_STATUS_ARROW,
WT_STATUS_MAXSLOT
};
--
2.42.0.402.gbe8243af7b.dirty
^ permalink raw reply related
* [RFC PATCH 4/5] add: add -t, --table flag for visual dry runs
From: Jacob Stopak @ 2023-10-20 18:39 UTC (permalink / raw)
To: git; +Cc: Jacob Stopak
In-Reply-To: <20231020183947.463882-1-jacob@initialcommit.io>
Signed-off-by: Jacob Stopak <jacob@initialcommit.io>
---
builtin/add.c | 46 ++++++++++++++++++------
builtin/commit.c | 2 +-
read-cache-ll.h | 9 ++++-
read-cache.c | 32 ++++++++++++++---
table.c | 92 +++++++++++++++++++++++++++++++++++++++++++-----
wt-status.c | 1 +
wt-status.h | 1 +
7 files changed, 157 insertions(+), 26 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index c27254a5cd..35ea1deda5 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -27,6 +27,7 @@
#include "strvec.h"
#include "submodule.h"
#include "add-interactive.h"
+#include "wt-status.h"
static const char * const builtin_add_usage[] = {
N_("git add [<options>] [--] <pathspec>..."),
@@ -221,7 +222,7 @@ N_("The following paths are ignored by one of your .gitignore files:\n");
static int verbose, show_only, ignored_too, refresh_only;
static int ignore_add_errors, intent_to_add, ignore_missing;
-static int warn_on_embedded_repo = 1;
+static int table_format, warn_on_embedded_repo = 1;
#define ADDREMOVE_DEFAULT 1
static int addremove = ADDREMOVE_DEFAULT;
@@ -264,6 +265,8 @@ static struct option builtin_add_options[] = {
N_("warn when adding an embedded repository")),
OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),
OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
+ OPT_SET_INT('t', "table", &table_format,
+ N_("show status in table format"), STATUS_FORMAT_TABLE),
OPT_END(),
};
@@ -322,7 +325,7 @@ static void check_embedded_repo(const char *path)
strbuf_release(&name);
}
-static int add_files(struct dir_struct *dir, int flags)
+static int add_files(struct dir_struct *dir, int flags, struct wt_status *status)
{
int i, exit_status = 0;
struct string_list matched_sparse_paths = STRING_LIST_INIT_NODUP;
@@ -345,7 +348,7 @@ static int add_files(struct dir_struct *dir, int flags)
dir->entries[i]->name);
continue;
}
- if (add_file_to_index(&the_index, dir->entries[i]->name, flags)) {
+ if (add_file_to_index_with_status(&the_index, dir->entries[i]->name, flags, status)) {
if (!ignore_add_errors)
die(_("adding files failed"));
exit_status = 1;
@@ -374,6 +377,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
int require_pathspec;
char *seen = NULL;
struct lock_file lock_file = LOCK_INIT;
+ struct wt_status status;
+ unsigned int progress_flag = 0;
git_config(add_config, NULL);
@@ -459,7 +464,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
(intent_to_add ? ADD_CACHE_INTENT : 0) |
(ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0) |
(!(addremove || take_worktree_changes)
- ? ADD_CACHE_IGNORE_REMOVAL : 0));
+ ? ADD_CACHE_IGNORE_REMOVAL : 0) |
+ (table_format ? ADD_CACHE_FORMAT_TABLE : 0));
if (repo_read_index_preload(the_repository, &pathspec, 0) < 0)
die(_("index file corrupt"));
@@ -551,15 +557,35 @@ int cmd_add(int argc, const char **argv, const char *prefix)
begin_odb_transaction();
- if (add_renormalize)
+ if (show_only && table_format) {
+ /* Prepare index and populate status */
+ wt_status_prepare(the_repository, &status);
+ git_config(git_default_config, &status);
+ repo_read_index(the_repository);
+ refresh_index(&the_index,
+ REFRESH_QUIET|REFRESH_UNMERGED|progress_flag,
+ &status.pathspec, NULL, NULL);
+ status.status_format = STATUS_FORMAT_TABLE;
+ status.show_branch = 0;
+ }
+
+ if (add_renormalize) {
exit_status |= renormalize_tracked_files(&pathspec, flags);
- else
- exit_status |= add_files_to_cache(the_repository, prefix,
+ } else {
+ exit_status |= add_files_to_cache_with_status(the_repository, prefix,
&pathspec, include_sparse,
- flags);
+ flags, &status);
+ }
- if (add_new_files)
- exit_status |= add_files(&dir, flags);
+ if (add_new_files) {
+ exit_status |= add_files(&dir, flags, &status);
+ }
+
+ if (show_only && table_format) {
+ wt_status_collect(&status);
+ wt_status_print(&status);
+ wt_status_collect_free_buffers(&status);
+ }
if (chmod_arg && pathspec.nr)
exit_status |= chmod_pathspec(&pathspec, chmod_arg[0], show_only);
diff --git a/builtin/commit.c b/builtin/commit.c
index 4338896dbf..33b15ef96e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -310,7 +310,7 @@ static void add_remove_files(struct string_list *list)
continue;
if (!lstat(p->string, &st)) {
- if (add_to_index(&the_index, p->string, &st, 0))
+ if (add_file_to_index(&the_index, p->string, 0))
die(_("updating files failed"));
} else
remove_file_from_index(&the_index, p->string);
diff --git a/read-cache-ll.h b/read-cache-ll.h
index 9a1a7edc5a..b1cee2c7ee 100644
--- a/read-cache-ll.h
+++ b/read-cache-ll.h
@@ -4,6 +4,7 @@
#include "hash-ll.h"
#include "hashmap.h"
#include "statinfo.h"
+#include "wt-status.h"
/*
* Basic data structures for the directory cache
@@ -395,6 +396,7 @@ int remove_file_from_index(struct index_state *, const char *path);
#define ADD_CACHE_IGNORE_ERRORS 4
#define ADD_CACHE_IGNORE_REMOVAL 8
#define ADD_CACHE_INTENT 16
+#define ADD_CACHE_FORMAT_TABLE 32
/*
* These two are used to add the contents of the file at path
* to the index, marking the working tree up-to-date by storing
@@ -404,7 +406,8 @@ int remove_file_from_index(struct index_state *, const char *path);
* the latter will do necessary lstat(2) internally before
* calling the former.
*/
-int add_to_index(struct index_state *, const char *path, struct stat *, int flags);
+int add_to_index(struct index_state *, const char *path, struct stat *, int flags, struct wt_status *status);
+int add_file_to_index_with_status(struct index_state *, const char *path, int flags, struct wt_status *status);
int add_file_to_index(struct index_state *, const char *path, int flags);
int chmod_index_entry(struct index_state *, struct cache_entry *ce, char flip);
@@ -475,6 +478,10 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
const struct pathspec *pathspec, int include_sparse,
int flags);
+int add_files_to_cache_with_status(struct repository *repo, const char *prefix,
+ const struct pathspec *pathspec, int include_sparse,
+ int flags, struct wt_status *status);
+
void overlay_tree_on_index(struct index_state *istate,
const char *tree_name, const char *prefix);
diff --git a/read-cache.c b/read-cache.c
index 080bd39713..e777cdb210 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -45,6 +45,8 @@
#include "csum-file.h"
#include "promisor-remote.h"
#include "hook.h"
+#include "wt-status.h"
+#include "string-list.h"
/* Mask for the name length in ce_flags in the on-disk index */
@@ -664,7 +666,7 @@ void set_object_name_for_intent_to_add_entry(struct cache_entry *ce)
oidcpy(&ce->oid, &oid);
}
-int add_to_index(struct index_state *istate, const char *path, struct stat *st, int flags)
+int add_to_index(struct index_state *istate, const char *path, struct stat *st, int flags, struct wt_status *status)
{
int namelen, was_same;
mode_t st_mode = st->st_mode;
@@ -672,6 +674,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
unsigned ce_option = CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE|CE_MATCH_RACY_IS_DIRTY;
int verbose = flags & (ADD_CACHE_VERBOSE | ADD_CACHE_PRETEND);
int pretend = flags & ADD_CACHE_PRETEND;
+ int table = flags & ADD_CACHE_FORMAT_TABLE;
int intent_only = flags & ADD_CACHE_INTENT;
int add_option = (ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE|
(intent_only ? ADD_CACHE_NEW_ONLY : 0));
@@ -760,17 +763,26 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
discard_cache_entry(ce);
return error(_("unable to add '%s' to index"), path);
}
- if (verbose && !was_same)
+ if (verbose && !was_same && !table)
printf("add '%s'\n", path);
+ if (table && pretend && !was_same) {
+ string_list_insert(&status->dry_run_added, path);
+ }
return 0;
}
-int add_file_to_index(struct index_state *istate, const char *path, int flags)
+int add_file_to_index_with_status(struct index_state *istate, const char *path, int flags, struct wt_status *status)
{
struct stat st;
if (lstat(path, &st))
die_errno(_("unable to stat '%s'"), path);
- return add_to_index(istate, path, &st, flags);
+ return add_to_index(istate, path, &st, flags, status);
+}
+
+int add_file_to_index(struct index_state *istate, const char *path, int flags)
+{
+ struct wt_status status;
+ return add_file_to_index_with_status(istate, path, flags, &status);
}
struct cache_entry *make_empty_cache_entry(struct index_state *istate, size_t len)
@@ -3872,6 +3884,7 @@ struct update_callback_data {
int include_sparse;
int flags;
int add_errors;
+ struct wt_status *status;
};
static int fix_unmerged_status(struct diff_filepair *p,
@@ -3914,7 +3927,7 @@ static void update_callback(struct diff_queue_struct *q,
die(_("unexpected diff status %c"), p->status);
case DIFF_STATUS_MODIFIED:
case DIFF_STATUS_TYPE_CHANGED:
- if (add_file_to_index(data->index, path, data->flags)) {
+ if (add_file_to_index_with_status(data->index, path, data->flags, data->status)) {
if (!(data->flags & ADD_CACHE_IGNORE_ERRORS))
die(_("updating files failed"));
data->add_errors++;
@@ -3935,6 +3948,14 @@ static void update_callback(struct diff_queue_struct *q,
int add_files_to_cache(struct repository *repo, const char *prefix,
const struct pathspec *pathspec, int include_sparse,
int flags)
+{
+ struct wt_status status;
+ return add_files_to_cache_with_status(repo, prefix, pathspec, include_sparse, flags, &status);
+}
+
+int add_files_to_cache_with_status(struct repository *repo, const char *prefix,
+ const struct pathspec *pathspec, int include_sparse,
+ int flags, struct wt_status *status)
{
struct update_callback_data data;
struct rev_info rev;
@@ -3943,6 +3964,7 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
data.index = repo->index;
data.include_sparse = include_sparse;
data.flags = flags;
+ data.status = status;
repo_init_revisions(repo, &rev, prefix);
setup_revisions(0, NULL, &rev, NULL);
diff --git a/table.c b/table.c
index 73751339da..a6fc660fec 100644
--- a/table.c
+++ b/table.c
@@ -65,18 +65,53 @@ static void build_table_entry(struct strbuf *buf, char *entry, int cols)
strbuf_addchars(buf, ' ', (cols / 3 - len - 1) / 2);
}
-static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
+static void add_arrow_to_entry(struct strbuf *buf, int add_after_entry)
+{
+ struct strbuf empty = STRBUF_INIT;
+ struct strbuf trimmed = STRBUF_INIT;
+ struct strbuf holder = STRBUF_INIT;
+ int len = strlen(buf->buf);
+
+ strbuf_addstr(&trimmed, buf->buf);
+ strbuf_trim(&trimmed);
+
+ if (!strbuf_cmp(&trimmed, &empty) && !add_after_entry) {
+ strbuf_reset(buf);
+ strbuf_addchars(buf, '-', len + 1);
+ } else if (add_after_entry) {
+ strbuf_rtrim(buf);
+ strbuf_addchars(buf, ' ', 1);
+ strbuf_addchars(buf, '-', len - strlen(buf->buf) + 1);
+ } else if (!add_after_entry) {
+ strbuf_ltrim(buf);
+ strbuf_addchars(&holder, '-', len - strlen(buf->buf) - 2);
+ strbuf_addchars(&holder, '>', 1);
+ strbuf_addchars(&holder, ' ', 1);
+ strbuf_addstr(&holder, buf->buf);
+ strbuf_reset(buf);
+ strbuf_addstr(buf, holder.buf);
+ }
+}
+
+static void print_table_body_line(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s, int hide_pipe)
{
printf(_("|"));
color_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), "%s", buf1->buf);
- printf(_("|"));
+ if (hide_pipe != 1 && hide_pipe != 3)
+ printf(_("|"));
color_fprintf(s->fp, color(WT_STATUS_CHANGED, s), "%s", buf2->buf);
- printf(_("|"));
+ if (hide_pipe != 2 && hide_pipe != 3)
+ printf(_("|"));
color_fprintf(s->fp, color(WT_STATUS_UPDATED, s), "%s", buf3->buf);
printf(_("|\n"));
}
-void build_and_draw_status_table(struct wt_status *s, int add_advice)
+static void print_table_body_line_(struct strbuf *buf1, struct strbuf *buf2, struct strbuf *buf3, struct wt_status *s)
+{
+ print_table_body_line(buf1, buf2, buf3, s, 0);
+}
+
+void build_and_draw_status_table(struct wt_status *s, int advice)
{
struct winsize w;
int cols;
@@ -84,7 +119,7 @@ void build_and_draw_status_table(struct wt_status *s, int add_advice)
struct strbuf table_col_entry_1 = STRBUF_INIT;
struct strbuf table_col_entry_2 = STRBUF_INIT;
struct strbuf table_col_entry_3 = STRBUF_INIT;
- struct string_list_item *item;
+ struct string_list_item *item, *item2;
/* Get terminal width */
ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
@@ -104,7 +139,7 @@ void build_and_draw_status_table(struct wt_status *s, int add_advice)
printf(_("%s\n"), table_border.buf);
printf(_("|%s|%s|%s|\n"), table_col_entry_1.buf, table_col_entry_2.buf, table_col_entry_3.buf);
- if (add_advice) {
+ if (advice) {
build_table_entry(&table_col_entry_1, "(stage: git add <file>)", cols);
build_table_entry(&table_col_entry_2, "(stage: git add <file>)", cols);
build_table_entry(&table_col_entry_3, "(unstage: git restore --staged <file>)", cols);
@@ -122,14 +157,38 @@ void build_and_draw_status_table(struct wt_status *s, int add_advice)
/* Draw table body */
for_each_string_list_item(item, &s->untracked) {
- build_table_entry(&table_col_entry_1, item->string, cols);
+ struct strbuf buf_1 = STRBUF_INIT;
+ struct strbuf buf_2 = STRBUF_INIT;
+ int is_arrow = 0;
+ strbuf_addstr(&buf_1, item->string);
+ build_table_entry(&table_col_entry_1, buf_1.buf, cols);
build_table_entry(&table_col_entry_2, "", cols);
build_table_entry(&table_col_entry_3, "", cols);
- print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+
+ for_each_string_list_item(item2, &s->dry_run_added) {
+ strbuf_reset(&buf_2);
+ strbuf_addstr(&buf_2, item2->string);
+ if (!strbuf_cmp(&buf_1, &buf_2)) {
+ build_table_entry(&table_col_entry_3, buf_1.buf, cols);
+ add_arrow_to_entry(&table_col_entry_1, 1);
+ add_arrow_to_entry(&table_col_entry_2, 0);
+ add_arrow_to_entry(&table_col_entry_3, 0);
+ is_arrow = 1;
+ }
+ }
+
+ if (!is_arrow)
+ print_table_body_line_(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+ else
+ print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s, 3);
}
for_each_string_list_item(item, &s->change) {
struct wt_status_change_data *d = item->util;
+ struct strbuf buf_1 = STRBUF_INIT;
+ struct strbuf buf_2 = STRBUF_INIT;
+ int is_arrow = 0;
+ strbuf_addstr(&buf_1, item->string);
if (d->worktree_status && d->index_status) {
build_table_entry(&table_col_entry_1, "", cols);
build_table_entry(&table_col_entry_2, item->string, cols);
@@ -138,12 +197,27 @@ void build_and_draw_status_table(struct wt_status *s, int add_advice)
build_table_entry(&table_col_entry_1, "", cols);
build_table_entry(&table_col_entry_2, item->string, cols);
build_table_entry(&table_col_entry_3, "", cols);
+
+ for_each_string_list_item(item2, &s->dry_run_added) {
+ strbuf_reset(&buf_2);
+ strbuf_addstr(&buf_2, item2->string);
+ if (!strbuf_cmp(&buf_1, &buf_2)) {
+ build_table_entry(&table_col_entry_3, buf_1.buf, cols);
+ add_arrow_to_entry(&table_col_entry_2, 1);
+ add_arrow_to_entry(&table_col_entry_3, 0);
+ is_arrow = 1;
+ }
+ }
} else if (d->index_status) {
build_table_entry(&table_col_entry_1, "", cols);
build_table_entry(&table_col_entry_2, "", cols);
build_table_entry(&table_col_entry_3, item->string, cols);
}
- print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+
+ if (!is_arrow)
+ print_table_body_line_(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s);
+ else
+ print_table_body_line(&table_col_entry_1, &table_col_entry_2, &table_col_entry_3, s, 2);
}
if (!s->untracked.nr && !s->change.nr) {
diff --git a/wt-status.c b/wt-status.c
index 62731859fe..975cfc01a5 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -153,6 +153,7 @@ void wt_status_prepare(struct repository *r, struct wt_status *s)
s->change.strdup_strings = 1;
s->untracked.strdup_strings = 1;
s->ignored.strdup_strings = 1;
+ s->dry_run_added.strdup_strings = 1;
s->show_branch = -1; /* unspecified */
s->show_stash = 0;
s->ahead_behind_flags = AHEAD_BEHIND_UNSPECIFIED;
diff --git a/wt-status.h b/wt-status.h
index 70a3b7a2e4..5d29c058c1 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -142,6 +142,7 @@ struct wt_status {
struct string_list change;
struct string_list untracked;
struct string_list ignored;
+ struct string_list dry_run_added;
uint32_t untracked_in_ms;
};
--
2.42.0.402.gbe8243af7b.dirty
^ permalink raw reply related
* [PATCH] git-push: more visibility for -q option
From: Michal Suchanek @ 2023-10-20 18:45 UTC (permalink / raw)
To: git; +Cc: Michal Suchanek
The -v option listed at the top as option al parameter while -q is not.
List -q alongside -v.
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
Documentation/git-push.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 5b4edaf4a8..003bc7d9ce 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -10,7 +10,7 @@ SYNOPSIS
--------
[verse]
'git push' [--all | --branches | --mirror | --tags] [--follow-tags] [--atomic] [-n | --dry-run] [--receive-pack=<git-receive-pack>]
- [--repo=<repository>] [-f | --force] [-d | --delete] [--prune] [-v | --verbose]
+ [--repo=<repository>] [-f | --force] [-d | --delete] [--prune] [-q | --quiet] [-v | --verbose]
[-u | --set-upstream] [-o <string> | --push-option=<string>]
[--[no-]signed|--signed=(true|false|if-asked)]
[--force-with-lease[=<refname>[:<expect>]] [--force-if-includes]]
--
2.42.0
^ permalink raw reply related
* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Dragan Simic @ 2023-10-20 18:48 UTC (permalink / raw)
To: Jacob Stopak; +Cc: git
In-Reply-To: <20231020183947.463882-1-jacob@initialcommit.io>
On 2023-10-20 20:39, Jacob Stopak wrote:
> This is a proposal / proof-of-concept for a new table-based output
> format for the git status command, and for dry runs (-n) of the git add
> command. This could be extended to create visual dry runs for other
> other commands like rm, mv, restore, stash, commit, and clean.
Huh, please don't get me wrong, but based on the examples provided
below, I really think that's only wasted screen estate, providing little
or no help in understanding the performed operations.
I appreciate your effort, but IMHO it makes little sense from the
usability standpoint.
> For some context, earlier this year I released a tool called Git-Sim
> (https://github.com/initialcommit-com/git-sim) which allows users to do
> visual dry runs of many Git commands, which are rendered as high
> quality
> output image files. Simulating commands like status, add, rm, mv,
> restore,
> stash, and commit creates a table with 3 columns to represent the way
> file
> changes "move around" as a result of the command being simulated.
>
> I've gotten positive feedback from users about this visual approach to
> simulating git commands, which is more intuitive than pure terminal
> text
> for both newer users to understand how git works and for visual people.
>
> As a result, I was thinking of ways to integrate these types of visual
> formats directly into Git. A table-based output format with colored
> highlighting for the commands mentioned above is low hanging fruit.
>
> Teach 'git status' the new -t, --table flag, which displays the status
> output in a 3-column table format, preserving terminology and color
> coding from the default git status "long output" format (note that the
> column headers are shortened here for the small width of this email,
> and
> also I just realized that the tables below might not look right on the
> mailing list due to the differing character width, but it looks correct
> in the terminal so please test there it's more fun anyway :D):
>
> $ git status -t
> -------------------------------------------------------------------------
> | Untracked files | Changes n...or commit | Changes t...committed
> |
> -------------------------------------------------------------------------
> | poiu | |
> |
> | status-table/ | |
> |
> | | | asdf
> |
> | | table.c |
> |
> | | wt-status.c |
> |
> -------------------------------------------------------------------------
>
> Teach 'git add' the new -t, --table flag to be used ONLY in combination
> with the '-n' flag for dry runs. Instead of simply printing out the
> added filenames, the full status table format is displayed, along with
> arrows that visually show how the added files are being moved around:
>
> $ git add -nt poiu wt-status.c
> -------------------------------------------------------------------------
> | Untracked files | Changes n...or commit | Changes t...committed
> |
> -------------------------------------------------------------------------
> | poiu -----------------------------------------> poiu
> |
> | status-table/ | |
> |
> | | | asdf
> |
> | | table.c |
> |
> | | wt-status.c ----------> wt-status.c
> |
> -------------------------------------------------------------------------
>
> Other notes:
>
> * The width of the table and columns are dynamically set based on the
> width of the terminal.
>
> * Long paths are shortened to include the maximum number of characters
> from both ends of the path that will fit, with a '...' in the middle.
>
> * Color coding matches the default output of 'git status', with
> untracked files and working dir mods in red, and staged changes in
> green. If needed, arrows are drawn in cyan.
>
> As stated above, the dry run version of the table format can be applied
> to various other commands like rm, mv, restore, stash, commit, and
> clean
> which all move file changes around in a way that can be represented in
> the table format. New columns may need to be added or arrows reversed
> to show changes moving in various directions. Note that some of these
> commands don't appear to have a dry run (-n) option yet, so it could be
> added for consistency (if not already in use) and for use with the new
> table format.
>
> Since this is an RFC patch series, I probably did some illegal and dumb
> things in my code changes just to get it into a demo-able state. I am a
> bit wary of having made changes to files like "read-cache.c" and
> "read-cache-ll.h" to pass in the wt_status info, and there are probably
> betters ways to do some other things too.
>
> Feedback on both the new format itself and the implementation is very
> much appreciated!
>
> Jacob Stopak (5):
> status: introduce -t, --table flag
> status: handle long paths with -t, --table flag
> status: add advice arg for -t, --table flag
> add: add -t, --table flag for visual dry runs
> add: set unique color for -t, --table arrows
>
> Makefile | 1 +
> builtin/add.c | 46 +++++++--
> builtin/commit.c | 4 +-
> read-cache-ll.h | 9 +-
> read-cache.c | 32 ++++++-
> table.c | 245 +++++++++++++++++++++++++++++++++++++++++++++++
> table.h | 6 ++
> wt-status.c | 74 +++++++++-----
> wt-status.h | 3 +
> 9 files changed, 378 insertions(+), 42 deletions(-)
> create mode 100644 table.c
> create mode 100644 table.h
^ permalink raw reply
* [PATCH v5 0/3] Trailer readability cleanups
From: Linus Arver via GitGitGadget @ 2023-10-20 19:01 UTC (permalink / raw)
To: git; +Cc: Glen Choo, Christian Couder, Phillip Wood, Jonathan Tan,
Linus Arver
In-Reply-To: <pull.1563.v4.git.1695709372.gitgitgadget@gmail.com>
These patches were created while digging into the trailer code to better
understand how it works, in preparation for making the trailer.{c,h} files
as small as possible to make them available as a library for external users.
This series was originally created as part of [1], but are sent here
separately because the changes here are arguably more subjective in nature.
These patches do not add or change any features. Instead, their goal is to
make the code easier to understand for new contributors (like myself), by
making various cleanups and improvements. Ultimately, my hope is that with
such cleanups, we are better positioned to make larger changes (especially
the broader libification effort, as in "Introduce Git Standard Library"
[2]).
Updates in v5
=============
* Patch 4 ("trailer: only use trailer_block_* variables if trailers were
found") has been dropped.
* Patch 2 returns early if "--no-divider" is true, avoiding unnecessary
loop iterations (thanks Jonathan).
* Added missing Reported-by trailer for Patch 3 (it was originally Glen's
idea to use offsets).
* Patch 3: Fixed typo in "trailer.h" that referred to an obsolete function
name ("find_true_end_of_input()", instead of
"find_end_of_log_message()").
Updates in v4
=============
* The first 3 patches in v3 were merged into 'master'. Necessarily, those 3
patches have been dropped.
* Patch 4 in v3 ("trailer: rename *_DEFAULT enums to *_UNSPECIFIED") has
been dropped, as well as Patch 9 in v3 ("trailer: make stack variable
names match field names"). These were dropped to simplify this series for
what I think is the more immediate, important change (see next bullet
point).
* Patches 5-8 in v3 are the only ones remaining in this series. They still
solely deal with --no-divider and trailer block start/end cleanups.
Updates in v3
=============
* Patches 4 and 6 (--no-divider and trailer block start/end cleanups) have
been reorganized to Patches 5-8. This ended up touching commit.c in a
minor way, but otherwise all of the changes here are cleanups and do not
change any behavior.
Updates in v2
=============
* Patch 1: Drop the use of a #define. Instead just use an anonymous struct
named internal.
* Patch 2: Don't free info out parameter inside parse_trailers(). Instead
free it from the caller, process_trailers(). Update comment in
parse_trailers().
* Patch 3: Reword commit message.
* Patch 4: Mention be3d654343 (commit: pass --no-divider to
interpret-trailers, 2023-06-17) in commit message.
* Added Patch 6 to make trailer_info use offsets for trailer_start and
trailer_end (thanks to Glen Choo for the suggestion).
[1]
https://lore.kernel.org/git/pull.1564.git.1691210737.gitgitgadget@gmail.com/T/#mb044012670663d8eb7a548924bbcc933bef116de
[2]
https://lore.kernel.org/git/20230627195251.1973421-1-calvinwan@google.com/
[3]
https://lore.kernel.org/git/pull.1149.git.1677143700.gitgitgadget@gmail.com/
[4]
https://lore.kernel.org/git/6b4cb31b17077181a311ca87e82464a1e2ad67dd.1686797630.git.gitgitgadget@gmail.com/
[5]
https://lore.kernel.org/git/pull.1563.git.1691211879.gitgitgadget@gmail.com/T/#m0131f9829c35d8e0103ffa88f07d8e0e43dd732c
Linus Arver (3):
commit: ignore_non_trailer computes number of bytes to ignore
trailer: find the end of the log message
trailer: use offsets for trailer_start/trailer_end
builtin/commit.c | 2 +-
builtin/merge.c | 2 +-
commit.c | 2 +-
commit.h | 4 +--
sequencer.c | 2 +-
trailer.c | 85 +++++++++++++++++++++++++++++-------------------
trailer.h | 10 +++---
7 files changed, 62 insertions(+), 45 deletions(-)
base-commit: bcb6cae2966cc407ca1afc77413b3ef11103c175
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1563%2Flistx%2Ftrailer-libification-prep-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1563/listx/trailer-libification-prep-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/1563
Range-diff vs v4:
1: 4ce5cf77005 = 1: 4ce5cf77005 commit: ignore_non_trailer computes number of bytes to ignore
2: c904caba7e1 ! 2: ce25420db29 trailer: find the end of the log message
@@ Commit message
the starting point which find_trailer_start() needs to start searching
backward to parse individual trailers (if any).
+ Helped-by: Jonathan Tan <jonathantanmy@google.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Linus Arver <linusa@google.com>
@@ trailer.c: static ssize_t last_line(const char *buf, size_t len)
+static size_t find_end_of_log_message(const char *input, int no_divider)
{
+ size_t end;
-+
const char *s;
- for (s = str; *s; s = next_line(s)) {
+ /* Assume the naive end of the input is already what we want. */
+ end = strlen(input);
+
++ if (no_divider) {
++ return end;
++ }
++
+ /* Optionally skip over any patch part ("---" line and below). */
+ for (s = input; *s; s = next_line(s)) {
const char *v;
- if (skip_prefix(s, "---", &v) && isspace(*v))
- return s - str;
-+ if (!no_divider && skip_prefix(s, "---", &v) && isspace(*v)) {
++ if (skip_prefix(s, "---", &v) && isspace(*v)) {
+ end = s - input;
+ break;
+ }
3: 796e47c1e5f ! 3: e3a7b150241 trailer: use offsets for trailer_start/trailer_end
@@ Commit message
more explicit about these offsets (that they are for the entire trailer
block including other trailers). Ditto for trailer_end.
+ Reported-by: Glen Choo <glencbz@gmail.com>
Signed-off-by: Linus Arver <linusa@google.com>
## sequencer.c ##
@@ trailer.h: int trailer_set_if_missing(enum trailer_if_missing *item, const char
- * input string.
+ * Offsets to the trailer block start and end positions in the input
+ * string. If no trailer block is found, these are both set to the
-+ * "true" end of the input, per find_true_end_of_input().
-+ *
-+ * NOTE: This will be changed so that these point to 0 in the next
-+ * patch if no trailers are found.
++ * "true" end of the input (find_end_of_log_message()).
*/
- const char *trailer_start, *trailer_end;
+ size_t trailer_block_start, trailer_block_end;
4: 64e1bd4e4be < -: ----------- trailer: only use trailer_block_* variables if trailers were found
--
gitgitgadget
^ permalink raw reply
* [PATCH v5 1/3] commit: ignore_non_trailer computes number of bytes to ignore
From: Linus Arver via GitGitGadget @ 2023-10-20 19:01 UTC (permalink / raw)
To: git
Cc: Glen Choo, Christian Couder, Phillip Wood, Jonathan Tan,
Linus Arver, Linus Arver
In-Reply-To: <pull.1563.v5.git.1697828495.gitgitgadget@gmail.com>
From: Linus Arver <linusa@google.com>
ignore_non_trailer() returns the _number of bytes_ that should be
ignored from the end of the log message. It does not by itself "ignore"
anything.
Rename this function to remove the leading "ignore" verb, to sound more
like a quantity than an action.
Signed-off-by: Linus Arver <linusa@google.com>
---
builtin/commit.c | 2 +-
builtin/merge.c | 2 +-
commit.c | 2 +-
commit.h | 4 ++--
trailer.c | 2 +-
5 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 7da5f924484..d1785d32db1 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -900,7 +900,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
strbuf_stripspace(&sb, '\0');
if (signoff)
- append_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0);
+ append_signoff(&sb, ignored_log_message_bytes(sb.buf, sb.len), 0);
if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)
die_errno(_("could not write commit template"));
diff --git a/builtin/merge.c b/builtin/merge.c
index 545da0c8a11..c654a29fe85 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -870,7 +870,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)
_(no_scissors_editor_comment), comment_line_char);
}
if (signoff)
- append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);
+ append_signoff(&msg, ignored_log_message_bytes(msg.buf, msg.len), 0);
write_merge_heads(remoteheads);
write_file_buf(git_path_merge_msg(the_repository), msg.buf, msg.len);
if (run_commit_hook(0 < option_edit, get_index_file(), NULL,
diff --git a/commit.c b/commit.c
index b3223478bc2..4440fbabb83 100644
--- a/commit.c
+++ b/commit.c
@@ -1769,7 +1769,7 @@ const char *find_commit_header(const char *msg, const char *key, size_t *out_len
* Returns the number of bytes from the tail to ignore, to be fed as
* the second parameter to append_signoff().
*/
-size_t ignore_non_trailer(const char *buf, size_t len)
+size_t ignored_log_message_bytes(const char *buf, size_t len)
{
size_t boc = 0;
size_t bol = 0;
diff --git a/commit.h b/commit.h
index 28928833c54..1cc872f225f 100644
--- a/commit.h
+++ b/commit.h
@@ -294,8 +294,8 @@ const char *find_header_mem(const char *msg, size_t len,
const char *find_commit_header(const char *msg, const char *key,
size_t *out_len);
-/* Find the end of the log message, the right place for a new trailer. */
-size_t ignore_non_trailer(const char *buf, size_t len);
+/* Find the number of bytes to ignore from the end of a log message. */
+size_t ignored_log_message_bytes(const char *buf, size_t len);
typedef int (*each_mergetag_fn)(struct commit *commit, struct commit_extra_header *extra,
void *cb_data);
diff --git a/trailer.c b/trailer.c
index b6de5d9cb2d..3c54b38a85a 100644
--- a/trailer.c
+++ b/trailer.c
@@ -928,7 +928,7 @@ continue_outer_loop:
/* Return the position of the end of the trailers. */
static size_t find_trailer_end(const char *buf, size_t len)
{
- return len - ignore_non_trailer(buf, len);
+ return len - ignored_log_message_bytes(buf, len);
}
static int ends_with_blank_line(const char *buf, size_t len)
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 2/3] trailer: find the end of the log message
From: Linus Arver via GitGitGadget @ 2023-10-20 19:01 UTC (permalink / raw)
To: git
Cc: Glen Choo, Christian Couder, Phillip Wood, Jonathan Tan,
Linus Arver, Linus Arver
In-Reply-To: <pull.1563.v5.git.1697828495.gitgitgadget@gmail.com>
From: Linus Arver <linusa@google.com>
Previously, trailer_info_get() computed the trailer block end position
by
(1) checking for the opts->no_divider flag and optionally calling
find_patch_start() to find the "patch start" location (patch_start), and
(2) calling find_trailer_end() to find the end of the trailer block
using patch_start as a guide, saving the return value into
"trailer_end".
The logic in (1) was awkward because the variable "patch_start" is
misleading if there is no patch in the input. The logic in (2) was
misleading because it could be the case that no trailers are in the
input (yet we are setting a "trailer_end" variable before even searching
for trailers, which happens later in find_trailer_start()). The name
"find_trailer_end" was misleading because that function did not look for
any trailer block itself --- instead it just computed the end position
of the log message in the input where the end of the trailer block (if
it exists) would be (because trailer blocks must always come after the
end of the log message).
Combine the logic in (1) and (2) together into find_patch_start() by
renaming it to find_end_of_log_message(). The end of the log message is
the starting point which find_trailer_start() needs to start searching
backward to parse individual trailers (if any).
Helped-by: Jonathan Tan <jonathantanmy@google.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Linus Arver <linusa@google.com>
---
trailer.c | 64 +++++++++++++++++++++++++++++++++++--------------------
1 file changed, 41 insertions(+), 23 deletions(-)
diff --git a/trailer.c b/trailer.c
index 3c54b38a85a..70c81fda710 100644
--- a/trailer.c
+++ b/trailer.c
@@ -809,21 +809,50 @@ static ssize_t last_line(const char *buf, size_t len)
}
/*
- * Return the position of the start of the patch or the length of str if there
- * is no patch in the message.
+ * Find the end of the log message as an offset from the start of the input
+ * (where callers of this function are interested in looking for a trailers
+ * block in the same input). We have to consider two categories of content that
+ * can come at the end of the input which we want to ignore (because they don't
+ * belong in the log message):
+ *
+ * (1) the "patch part" which begins with a "---" divider and has patch
+ * information (like the output of git-format-patch), and
+ *
+ * (2) any trailing comment lines, blank lines like in the output of "git
+ * commit -v", or stuff below the "cut" (scissor) line.
+ *
+ * As a formula, the situation looks like this:
+ *
+ * INPUT = LOG MESSAGE + IGNORED
+ *
+ * where IGNORED can be either of the two categories described above. It may be
+ * that there is nothing to ignore. Now it may be the case that the LOG MESSAGE
+ * contains a trailer block, but that's not the concern of this function.
*/
-static size_t find_patch_start(const char *str)
+static size_t find_end_of_log_message(const char *input, int no_divider)
{
+ size_t end;
const char *s;
- for (s = str; *s; s = next_line(s)) {
+ /* Assume the naive end of the input is already what we want. */
+ end = strlen(input);
+
+ if (no_divider) {
+ return end;
+ }
+
+ /* Optionally skip over any patch part ("---" line and below). */
+ for (s = input; *s; s = next_line(s)) {
const char *v;
- if (skip_prefix(s, "---", &v) && isspace(*v))
- return s - str;
+ if (skip_prefix(s, "---", &v) && isspace(*v)) {
+ end = s - input;
+ break;
+ }
}
- return s - str;
+ /* Skip over other ignorable bits. */
+ return end - ignored_log_message_bytes(input, end);
}
/*
@@ -925,12 +954,6 @@ continue_outer_loop:
return len;
}
-/* Return the position of the end of the trailers. */
-static size_t find_trailer_end(const char *buf, size_t len)
-{
- return len - ignored_log_message_bytes(buf, len);
-}
-
static int ends_with_blank_line(const char *buf, size_t len)
{
ssize_t ll = last_line(buf, len);
@@ -1101,7 +1124,7 @@ void process_trailers(const char *file,
void trailer_info_get(struct trailer_info *info, const char *str,
const struct process_trailer_options *opts)
{
- int patch_start, trailer_end, trailer_start;
+ int end_of_log_message, trailer_start;
struct strbuf **trailer_lines, **ptr;
char **trailer_strings = NULL;
size_t nr = 0, alloc = 0;
@@ -1109,16 +1132,11 @@ void trailer_info_get(struct trailer_info *info, const char *str,
ensure_configured();
- if (opts->no_divider)
- patch_start = strlen(str);
- else
- patch_start = find_patch_start(str);
-
- trailer_end = find_trailer_end(str, patch_start);
- trailer_start = find_trailer_start(str, trailer_end);
+ end_of_log_message = find_end_of_log_message(str, opts->no_divider);
+ trailer_start = find_trailer_start(str, end_of_log_message);
trailer_lines = strbuf_split_buf(str + trailer_start,
- trailer_end - trailer_start,
+ end_of_log_message - trailer_start,
'\n',
0);
for (ptr = trailer_lines; *ptr; ptr++) {
@@ -1141,7 +1159,7 @@ void trailer_info_get(struct trailer_info *info, const char *str,
info->blank_line_before_trailer = ends_with_blank_line(str,
trailer_start);
info->trailer_start = str + trailer_start;
- info->trailer_end = str + trailer_end;
+ info->trailer_end = str + end_of_log_message;
info->trailers = trailer_strings;
info->trailer_nr = nr;
}
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 3/3] trailer: use offsets for trailer_start/trailer_end
From: Linus Arver via GitGitGadget @ 2023-10-20 19:01 UTC (permalink / raw)
To: git
Cc: Glen Choo, Christian Couder, Phillip Wood, Jonathan Tan,
Linus Arver, Linus Arver
In-Reply-To: <pull.1563.v5.git.1697828495.gitgitgadget@gmail.com>
From: Linus Arver <linusa@google.com>
Previously these fields in the trailer_info struct were of type "const
char *" and pointed to positions in the input string directly (to the
start and end positions of the trailer block).
Use offsets to make the intended usage less ambiguous. We only need to
reference the input string in format_trailer_info(), so update that
function to take a pointer to the input.
While we're at it, rename trailer_start to trailer_block_start to be
more explicit about these offsets (that they are for the entire trailer
block including other trailers). Ditto for trailer_end.
Reported-by: Glen Choo <glencbz@gmail.com>
Signed-off-by: Linus Arver <linusa@google.com>
---
sequencer.c | 2 +-
trailer.c | 29 ++++++++++++++---------------
trailer.h | 10 +++++-----
3 files changed, 20 insertions(+), 21 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index d584cac8ed9..8707a92204f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -345,7 +345,7 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
if (ignore_footer)
sb->buf[sb->len - ignore_footer] = saved_char;
- if (info.trailer_start == info.trailer_end)
+ if (info.trailer_block_start == info.trailer_block_end)
return 0;
for (i = 0; i < info.trailer_nr; i++)
diff --git a/trailer.c b/trailer.c
index 70c81fda710..f7dc7c4c008 100644
--- a/trailer.c
+++ b/trailer.c
@@ -859,7 +859,7 @@ static size_t find_end_of_log_message(const char *input, int no_divider)
* Return the position of the first trailer line or len if there are no
* trailers.
*/
-static size_t find_trailer_start(const char *buf, size_t len)
+static size_t find_trailer_block_start(const char *buf, size_t len)
{
const char *s;
ssize_t end_of_title, l;
@@ -1075,7 +1075,6 @@ void process_trailers(const char *file,
LIST_HEAD(head);
struct strbuf sb = STRBUF_INIT;
struct trailer_info info;
- size_t trailer_end;
FILE *outfile = stdout;
ensure_configured();
@@ -1086,11 +1085,10 @@ void process_trailers(const char *file,
outfile = create_in_place_tempfile(file);
parse_trailers(&info, sb.buf, &head, opts);
- trailer_end = info.trailer_end - sb.buf;
/* Print the lines before the trailers */
if (!opts->only_trailers)
- fwrite(sb.buf, 1, info.trailer_start - sb.buf, outfile);
+ fwrite(sb.buf, 1, info.trailer_block_start, outfile);
if (!opts->only_trailers && !info.blank_line_before_trailer)
fprintf(outfile, "\n");
@@ -1112,7 +1110,7 @@ void process_trailers(const char *file,
/* Print the lines after the trailers as is */
if (!opts->only_trailers)
- fwrite(sb.buf + trailer_end, 1, sb.len - trailer_end, outfile);
+ fwrite(sb.buf + info.trailer_block_end, 1, sb.len - info.trailer_block_end, outfile);
if (opts->in_place)
if (rename_tempfile(&trailers_tempfile, file))
@@ -1124,7 +1122,7 @@ void process_trailers(const char *file,
void trailer_info_get(struct trailer_info *info, const char *str,
const struct process_trailer_options *opts)
{
- int end_of_log_message, trailer_start;
+ size_t end_of_log_message = 0, trailer_block_start = 0;
struct strbuf **trailer_lines, **ptr;
char **trailer_strings = NULL;
size_t nr = 0, alloc = 0;
@@ -1133,10 +1131,10 @@ void trailer_info_get(struct trailer_info *info, const char *str,
ensure_configured();
end_of_log_message = find_end_of_log_message(str, opts->no_divider);
- trailer_start = find_trailer_start(str, end_of_log_message);
+ trailer_block_start = find_trailer_block_start(str, end_of_log_message);
- trailer_lines = strbuf_split_buf(str + trailer_start,
- end_of_log_message - trailer_start,
+ trailer_lines = strbuf_split_buf(str + trailer_block_start,
+ end_of_log_message - trailer_block_start,
'\n',
0);
for (ptr = trailer_lines; *ptr; ptr++) {
@@ -1157,9 +1155,9 @@ void trailer_info_get(struct trailer_info *info, const char *str,
strbuf_list_free(trailer_lines);
info->blank_line_before_trailer = ends_with_blank_line(str,
- trailer_start);
- info->trailer_start = str + trailer_start;
- info->trailer_end = str + end_of_log_message;
+ trailer_block_start);
+ info->trailer_block_start = trailer_block_start;
+ info->trailer_block_end = end_of_log_message;
info->trailers = trailer_strings;
info->trailer_nr = nr;
}
@@ -1174,6 +1172,7 @@ void trailer_info_release(struct trailer_info *info)
static void format_trailer_info(struct strbuf *out,
const struct trailer_info *info,
+ const char *msg,
const struct process_trailer_options *opts)
{
size_t origlen = out->len;
@@ -1183,8 +1182,8 @@ static void format_trailer_info(struct strbuf *out,
if (!opts->only_trailers && !opts->unfold && !opts->filter &&
!opts->separator && !opts->key_only && !opts->value_only &&
!opts->key_value_separator) {
- strbuf_add(out, info->trailer_start,
- info->trailer_end - info->trailer_start);
+ strbuf_add(out, msg + info->trailer_block_start,
+ info->trailer_block_end - info->trailer_block_start);
return;
}
@@ -1238,7 +1237,7 @@ void format_trailers_from_commit(struct strbuf *out, const char *msg,
struct trailer_info info;
trailer_info_get(&info, msg, opts);
- format_trailer_info(out, &info, opts);
+ format_trailer_info(out, &info, msg, opts);
trailer_info_release(&info);
}
diff --git a/trailer.h b/trailer.h
index ab2cd017567..1644cd05f60 100644
--- a/trailer.h
+++ b/trailer.h
@@ -32,16 +32,16 @@ int trailer_set_if_missing(enum trailer_if_missing *item, const char *value);
struct trailer_info {
/*
* True if there is a blank line before the location pointed to by
- * trailer_start.
+ * trailer_block_start.
*/
int blank_line_before_trailer;
/*
- * Pointers to the start and end of the trailer block found. If there
- * is no trailer block found, these 2 pointers point to the end of the
- * input string.
+ * Offsets to the trailer block start and end positions in the input
+ * string. If no trailer block is found, these are both set to the
+ * "true" end of the input (find_end_of_log_message()).
*/
- const char *trailer_start, *trailer_end;
+ size_t trailer_block_start, trailer_block_end;
/*
* Array of trailers found.
--
gitgitgadget
^ permalink raw reply related
* Why sometimes branch merging is associated with this commit: Merge remote-tracking branch 'remotes/p4/HEAD'
From: Yuri @ 2023-10-20 19:46 UTC (permalink / raw)
To: Git Mailing List
I use git-p4 to use the perforce repository through git.
There are 3 branches:
* master
remotes/p4/HEAD -> p4/master
remotes/p4/master
git-p4 syncs remotes/p4/HEAD with the perforce repository.
Then the user (me) needs to merge remotes/p4/HEAD into master.
Initially such merge doesn't cause "Merge remote-tracking branch
'remotes/p4/HEAD'" commits.
But then, after several cycles of submit/sync something happens, and git
forces me to commit with the "Merge remote-tracking branch
'remotes/p4/HEAD'" comment.
What makes the merge from remotes/p4/HEAD into master to require "Merge
remote-tracking branch 'remotes/p4/HEAD'"?
What does this mean?
What is changed in remotes/p4/HEAD or master that later requires this?
How to eliminate the need for "Merge remote-tracking branch
'remotes/p4/HEAD'"?
Thanks,
Yuri
^ permalink raw reply
* Re: [PATCH 05/11] t: convert tests to not access symrefs via the filesystem
From: Junio C Hamano @ 2023-10-20 19:52 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Han-Wen Nienhuys
In-Reply-To: <1ac120368c6cd995841c28bde7542e882ec7b04f.1697607222.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> @@ -164,9 +164,9 @@ test_expect_success 'rev-parse skips symref pointing to broken name' '
> test_expect_success 'for-each-ref emits warnings for broken names' '
> test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
> - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
> + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref &&
I am of two minds here. While it certainly smells nicer because we
can test ref backends other than the files backend with this change,
we are forcing all ref backends to support creating a symbolic ref
with invalid name, because otherwise "test-tool" would not be able
to do this. Newer more database-oriented ref backends should be
allowed to implement their file format in which it is imposssible to
store such a bad name, but this makes it impossible.
I guess it is OK, because we would introduce some new prerequisite
(i.e. REF_BACKEND_ALLOWS_BROKEN_REFS) to skip this test on certain
ref backend where making invalid refs is impossible.
Other kind of changes in this patch, e.g., ...
> @@ -315,7 +325,9 @@ test_expect_success 'defaulted HEAD uses remote branch if available' '
> git -c init.defaultBranch=branchwithstuff -c protocol.version=2 \
> clone "file://$(pwd)/file_unborn_parent" \
> file_unborn_child 2>stderr &&
> - grep "refs/heads/branchwithstuff" file_unborn_child/.git/HEAD &&
> + echo "refs/heads/branchwithstuff" >expect &&
> + git -C file_unborn_child symbolic-ref HEAD >actual &&
> + test_cmp expect actual &&
> test_path_is_file file_unborn_child/stuff.t &&
> ! grep "warning:" stderr
> '
> diff --git a/t/t9133-git-svn-nested-git-repo.sh b/t/t9133-git-svn-nested-git-repo.sh
> index d8d536269cf..8ca24670acb 100755
> --- a/t/t9133-git-svn-nested-git-repo.sh
> +++ b/t/t9133-git-svn-nested-git-repo.sh
> @@ -11,7 +11,7 @@ test_expect_success 'setup repo with a git repo inside it' '
> (
> cd s &&
> git init &&
> - test -f .git/HEAD &&
> + git symbolic-ref HEAD &&
> > .git/a &&
> echo a > a &&
> svn_cmd add .git a &&
... all looked sensible, though.
Thanks.
^ permalink raw reply
* Re: [PATCH] git-push: more visibility for -q option
From: Eric Sunshine @ 2023-10-20 20:05 UTC (permalink / raw)
To: Michal Suchanek; +Cc: git
In-Reply-To: <20231020184627.14336-1-msuchanek@suse.de>
On Fri, Oct 20, 2023 at 2:46 PM Michal Suchanek <msuchanek@suse.de> wrote:
> The -v option listed at the top as option al parameter while -q is not.
s/option al/optional/
> List -q alongside -v.
>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
^ permalink raw reply
* Re: [PATCH] git-push: more visibility for -q option
From: Junio C Hamano @ 2023-10-20 20:08 UTC (permalink / raw)
To: Michal Suchanek; +Cc: git
In-Reply-To: <20231020184627.14336-1-msuchanek@suse.de>
Michal Suchanek <msuchanek@suse.de> writes:
> The -v option listed at the top as option al parameter while -q is not.
"as option al parameter" - ECANNOTPARSE. Probably
The `-v` option is shown in the SYNOPSIS section near the top,
but `-q` is not shown anywhere there.
or something, I think. I agree showing it next to "-v" would make
the most sense.
>
> List -q alongside -v.
>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> Documentation/git-push.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> index 5b4edaf4a8..003bc7d9ce 100644
> --- a/Documentation/git-push.txt
> +++ b/Documentation/git-push.txt
> @@ -10,7 +10,7 @@ SYNOPSIS
> --------
> [verse]
> 'git push' [--all | --branches | --mirror | --tags] [--follow-tags] [--atomic] [-n | --dry-run] [--receive-pack=<git-receive-pack>]
> - [--repo=<repository>] [-f | --force] [-d | --delete] [--prune] [-v | --verbose]
> + [--repo=<repository>] [-f | --force] [-d | --delete] [--prune] [-q | --quiet] [-v | --verbose]
Yup, the change makes sense. We may want to wrap the first line to
a more reasonable length in a separate commit, and when that
happens, we probably would want to start [-v] [-q] on a separate
line as well, but for now this would do.
^ permalink raw reply
* Re: Regression: git send-email fails with "Use of uninitialized value $address" + "unable to extract a valid address"
From: Michael Strawbridge @ 2023-10-20 21:06 UTC (permalink / raw)
To: Uwe Kleine-König, git; +Cc: Luben Tuikov, entwicklung
In-Reply-To: <20231020100442.an47wwsti2d4zeyx@pengutronix.de>
On 10/20/23 06:04, Uwe Kleine-König wrote:
> hello,
>
> On Fri, Oct 13, 2023 at 04:14:37PM +0200, Uwe Kleine-König wrote:
>> Hello,
>>
>> $ git send-email --to 'A B <a@b.org>, C D <c@d.org>' lala.patch
>> Use of uninitialized value $address in sprintf at /usr/lib/git-core/git-send-email line 1172.
>> error: unable to extract a valid address from:
>>
>> This happens for me with git 2.42.0 and also on master (59167d7d09fd, "The seventeenth batch").
>>
>> Bisection points at
>>
>> a8022c5f7b67 ("send-email: expose header information to git-send-email's sendemail-validate hook")
>>
>> I didn't try to understand that change and fix the problem.
>
> Another (similar?) problem with non-ascii-chars:
>
> $ git send-email --to 'Will Deacon <will@kernel.org>' --to 'Krzysztof Wilczyński <kw@linux.com>' --to 'Lorenzo Pieralisi <lpieralisi@kernel.org>' --cc 'Rob Herring <robh@kernel.org>' --to 'Bjorn Helgaas <bhelgaas@google.com>' --cc 'linux-pci@vger.kernel.org' --cc kernel@pengutronix.de -1 --base=@~
> Use of uninitialized value $address in sprintf at /home/uwe/gsrc/git/git-send-email line 1162.
> error: unable to extract a valid address from:
>
> Bisection points to the same commit, when dropping ń in Krzysztof's
> name, it works fine.
>
This is interesting. Thanks for reporting it. If you are able, could you please try the patches found in the below threads:
- https://public-inbox.org/git/20230918212004.GC2163162@coredump.intra.peff.net/T/#mae64003cbb72f015bf5c0c04216524fcb6bb8d09
- https://public-inbox.org/git/f5c6a72b-f888-4d43-8be8-3ce2c878c669@gmail.com/T/#mca12dc95ccfd3ce2b94e7752ebaae9891201084f
Thanks,
Michael
> Best regards
> Uwe
>
^ permalink raw reply
* Re: [PATCH v5 2/3] trailer: find the end of the log message
From: Junio C Hamano @ 2023-10-20 21:29 UTC (permalink / raw)
To: Linus Arver via GitGitGadget
Cc: git, Glen Choo, Christian Couder, Phillip Wood, Jonathan Tan,
Linus Arver
In-Reply-To: <ce25420db29c9953095db652584dbed4e35d67ad.1697828495.git.gitgitgadget@gmail.com>
"Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Linus Arver <linusa@google.com>
>
> Previously, trailer_info_get() computed the trailer block end position
> by
>
> (1) checking for the opts->no_divider flag and optionally calling
> find_patch_start() to find the "patch start" location (patch_start), and
> (2) calling find_trailer_end() to find the end of the trailer block
> using patch_start as a guide, saving the return value into
> "trailer_end".
>
> The logic in (1) was awkward because the variable "patch_start" is
> misleading if there is no patch in the input. The logic in (2) was
> misleading because it could be the case that no trailers are in the
> input (yet we are setting a "trailer_end" variable before even searching
> for trailers, which happens later in find_trailer_start()). The name
> "find_trailer_end" was misleading because that function did not look for
> any trailer block itself --- instead it just computed the end position
> of the log message in the input where the end of the trailer block (if
> it exists) would be (because trailer blocks must always come after the
> end of the log message).
>
> Combine the logic in (1) and (2) together into find_patch_start() by
> renaming it to find_end_of_log_message(). The end of the log message is
> the starting point which find_trailer_start() needs to start searching
> backward to parse individual trailers (if any).
>
> Helped-by: Jonathan Tan <jonathantanmy@google.com>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Linus Arver <linusa@google.com>
> ---
> trailer.c | 64 +++++++++++++++++++++++++++++++++++--------------------
> 1 file changed, 41 insertions(+), 23 deletions(-)
>
> diff --git a/trailer.c b/trailer.c
> index 3c54b38a85a..70c81fda710 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -809,21 +809,50 @@ static ssize_t last_line(const char *buf, size_t len)
> }
>
> /*
> - * Return the position of the start of the patch or the length of str if there
> - * is no patch in the message.
> + * Find the end of the log message as an offset from the start of the input
> + * (where callers of this function are interested in looking for a trailers
> + * block in the same input). We have to consider two categories of content that
> + * can come at the end of the input which we want to ignore (because they don't
> + * belong in the log message):
> + *
> + * (1) the "patch part" which begins with a "---" divider and has patch
> + * information (like the output of git-format-patch), and
> + *
> + * (2) any trailing comment lines, blank lines like in the output of "git
> + * commit -v", or stuff below the "cut" (scissor) line.
> + *
> + * As a formula, the situation looks like this:
> + *
> + * INPUT = LOG MESSAGE + IGNORED
> + *
> + * where IGNORED can be either of the two categories described above. It may be
> + * that there is nothing to ignore. Now it may be the case that the LOG MESSAGE
> + * contains a trailer block, but that's not the concern of this function.
> */
> -static size_t find_patch_start(const char *str)
> +static size_t find_end_of_log_message(const char *input, int no_divider)
> {
> + size_t end;
> const char *s;
>
> - for (s = str; *s; s = next_line(s)) {
> + /* Assume the naive end of the input is already what we want. */
> + end = strlen(input);
> +
> + if (no_divider) {
> + return end;
> + }
OK. The early return may make sense, as we are essentially
declaring that everything is the "INPUT (= message + ignored)".
You do not need {braces} around a single-statement block, though.
Other than that, I didn't find anything quesionable in any of the
patches in this round. Looking good.
Thanks.
^ permalink raw reply
* Re: [RESEND] git-rebase.txt: rewrite docu for fixup/squash (again)
From: Marc Branchaud @ 2023-10-20 21:40 UTC (permalink / raw)
To: Oswald Buddenhagen, git
Cc: Junio C Hamano, Phillip Wood, Christian Couder, Charvi Mendiratta
In-Reply-To: <20231020092707.917514-1-oswald.buddenhagen@gmx.de>
On 2023-10-20 05:27, Oswald Buddenhagen wrote:
> Create a clear top-down structure which makes it hopefully unambiguous
> what happens when.
>
> Also mention the timestamp along with the author - this is primarily
> meant to include the keywords somebody might be searching for, like I
> did a year ago.
>
> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
I think this is a definite improvement to the text. Thanks for working
on this! I do have a few suggestions, below.
> ---
> Cc: Junio C Hamano <gitster@pobox.com>
> Cc: Phillip Wood <phillip.wood123@gmail.com>
> Cc: Christian Couder <christian.couder@gmail.com>
> Cc: Charvi Mendiratta <charvi077@gmail.com>
> Cc: Marc Branchaud <marcnarc@xiplink.com>
> ---
> Documentation/git-rebase.txt | 29 +++++++++++++++--------------
> 1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index e7b39ad244..857e025361 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -890,20 +890,21 @@ command "pick" with the command "reword".
> To drop a commit, replace the command "pick" with "drop", or just
> delete the matching line.
>
> -If you want to fold two or more commits into one, replace the command
> -"pick" for the second and subsequent commits with "squash" or "fixup".
> -If the commits had different authors, the folded commit will be
> -attributed to the author of the first commit. The suggested commit
> -message for the folded commit is the concatenation of the first
> -commit's message with those identified by "squash" commands, omitting the
> -messages of commits identified by "fixup" commands, unless "fixup -c"
> -is used. In that case the suggested commit message is only the message
> -of the "fixup -c" commit, and an editor is opened allowing you to edit
> -the message. The contents (patch) of the "fixup -c" commit are still
> -incorporated into the folded commit. If there is more than one "fixup -c"
> -commit, the message from the final one is used. You can also use
> -"fixup -C" to get the same behavior as "fixup -c" except without opening
> -an editor.
> +If you want to fold two or more commits into one (that is, to combine
> +their contents/patches), replace the command "pick" for the second and
> +subsequent commits with "squash" or "fixup".
> +The commit message for the folded commit is the concatenation of the
> +first commit's message with those identified by "squash" commands,
I think the original text's "those identified by" is a bit vague: Does
"those" mean "messages" or "commits"? The sentence reads like "those"
stands for "messages", but then of course you don't identify *messages*
with "squash" commands.
Maybe
those from commits identified by
?
> +omitting the messages of commits identified by "fixup" commands, unless
> +"fixup -c" is used. In the latter case, the message is obtained only
> +from the "fixup -c" commit
I think it's worth emphasizing that "fixup -c" also drops the message
from the initial "pick" commit as well as all the "squash" commits (I
presume that's the case, I haven't tried it [1]).
Maybe emphasize the word "only" in the sentence (i.e. spell it as
'only'). To really drive the point home it could say something like
obtained 'only' from the "fixup -c" commit, dropping the
messages of all the other involved commits
> (having more than one "fixup -c" commit
> +makes no sense, and only the message from the last one is used).
"Makes no sense" seems a bit opinionated (although I agree with the
sentiment). Also, you can legitimately have more than one "fixup -c" in
the overall instruction set, as long as there's at least one "pick"
command in between, e.g.
pick 1111beef
fixup 2222f00d
fixup -c 3333ab1e
pick 4444d00d
fixup 5555feed
fixup -c 6666dead
How about
(if more than one "fixup -c" command appears in a sequence
of "fixup" and "squash" commands, only the message from
the last "fixup -c" commit is used)
Thanks,
M.
[1] Makes me wonder if rebase should also support "squash -c"...
^ permalink raw reply
* Re: [PATCH 0/3] some send-email --compose fixes
From: Junio C Hamano @ 2023-10-20 21:42 UTC (permalink / raw)
To: Jeff King; +Cc: Michael Strawbridge, Bagas Sanjaya, Git Mailing List
In-Reply-To: <20231020100343.GA2194322@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> [culling the rather large cc, as we moving off the original topic]
>
> On Fri, Oct 20, 2023 at 03:14:03AM -0400, Jeff King wrote:
>
>> and there's your perl array ref (from the square brackets, which are
>> necessary because we're sticking it in a hash value). But even before
>> your patch, this seems to end up as garbage. The code which reads
>> $parsed_line does not dereference the array.
>>
>> The patch to fix it is only a few lines (well, more than that with some
>> light editorializing in the comments):
>
> So here's the fix in a cleaned up form, guided by my own comments from
> earlier. ;) I think this is actually all orthogonal to the patch you are
> working on, so yours could either go on top or just be applied
> separately.
>
> [1/3]: doc/send-email: mention handling of "reply-to" with --compose
> [2/3]: Revert "send-email: extract email-parsing code into a subroutine"
> [3/3]: send-email: handle to/cc/bcc from --compose message
Nice.
With the approach suggested to move the validation down to where the
necessary addresses are already all defined, Michael observed "whoa,
why am I getting stringified array ref?". If that is the only issue
in the approach, queuing these three patches first and then have
Michael's fix on top of them sounds like the cleanest thing to do.
Will queue on top of v2.42.0 to help those who may want to backport
these to the maintenance track.
Thanks.
^ 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