* Re: GFW fails with ST3 on Windows 11
From: Konstantin Khomoutov @ 2023-10-27 12:29 UTC (permalink / raw)
To: git
In-Reply-To: <cd248c66-fc71-4783-9195-02c8811744b8@app.fastmail.com>
On Fri, Oct 27, 2023 at 02:31:32PM +0700, John wrote:
> I have been using Sublime Text 3 as the editor on Git for Windows for years,
> on Windows 10. I recently purchased a Windows 11 machine. On that machine,
> when I give GFW the following command, I get the response shown:
>
> $ git commit -a
> […]
> hint: Waiting for your editor to close the file… C:\Program Files\Sublime Text 3\sublime_text.exe: C:Program: command not found
> error: There was a problem with the editor ‘C:\Program Files\Sublime Text 3\sublime_text.exe’.
> Please supply the message using either -m or -F option.
>
> Does anyone know of a fix? To repeat, it’s only a problem on Windows 11.
What if you change the contents of the GIT_EDITOR (or EDITOR - whichever is
set) environment variable to contain forward slashes? That is, for the
pathname to read "C:/Program Files/..."?
If that does not work, what if you replace "C:/" with "/c/" - that is, for the
pathname to read "/c/Program files/..."?
Please note that depending on where this variable is set (in the "System"
block of setting or "User") you might need to reboot or relogin,
correspondingly in order for the setting to be applied to your environment.
Also please check the core.editor configuration setting - it might be set,
and then you should try to modify it instead [3].
While we're at it, such issues are better discussed on the dedicated mailing
list for GfW [1]. Also check the project's issue tracker [2].
1. https://groups.google.com/g/git-for-windows/
2. https://github.com/git-for-windows/git/issues/
3. https://github.com/git-for-windows/git/issues/782
^ permalink raw reply
* Re: using oldest date when squashing commits
From: Marc Branchaud @ 2023-10-27 12:34 UTC (permalink / raw)
To: Johannes Sixt, Junio C Hamano, Phillip Wood
Cc: Oswald Buddenhagen, phillip.wood, git
In-Reply-To: <59731c05-c3f6-4815-8411-783bb1c2aac4@kdbg.org>
On 2023-10-24 17:19, Johannes Sixt wrote:
> Am 24.10.23 um 19:30 schrieb Junio C Hamano:
>> Phillip Wood <phillip.wood123@gmail.com> writes:
>>> "fixup -c/-C" were conceived as a way to reword a commit message at
>>> the same time as optionally fixing up the commit's content.
>>
>> Yup, it still is a "fix", meaning the identity and the spirit of the
>> commit being fixed are unchanged.
>
> That's a pitty, because that is not at all what *I* use "fixup -C" for.
> To update the commit message, I use "squash" (or occasionally "reword").
> I use "fixup -C" after the following events:
>
> 1. Commit unfinished changes for whatever reason. Usually the commit
> message just says "WIP <topic>" because that's what it is.
> 2. Make a fixup commit for an earlier commit because doing the fixup now
> gets it out of the way, and often delaying it until after the completed
> change would cause merge conflicts.
> 3. Complete the WIP including the commit message.
>
> I would now use "fixup -C" on commit 3, because its metadata reflects
> reality more accurately than that of 1. Commit 3 often comes days after 1.
Speaking of the metadata ...
I never use "fixup -C" (or -c), but I do use squash/fixup a lot. I find
that I would prefer it if Git used the most recent Author date from the
set of commits being combined, rather than preserving the picked
commit's Author date. Sometimes it takes quite a while for me to get a
piece of work sorted out, and I would rather have the Author date in the
end-result commit reflect the work's completion time than its initiation
time.
The current behaviour means that when scanning through commits with
tools like gitk (which shows just the Author date in its list of
commits) I'll often see what I feel are inaccurate or confusing dates
there, and I use the Committer date (a bit less convenient in gitk) to
figure out when the work was actually "done". (Although the span
between the Author date from the start of the work and the Committer
date from the end of the work would roughly reflect how long the work
took to complete, I don't use Git for that kind of information.)
Anyway, this is a minor itch for me that I've never felt the need to
scratch. I just thought I'd mention it since the topic is being discussed.
M.
^ permalink raw reply
* Re: GFW fails with ST3 on Windows 11
From: Konstantin Khomoutov @ 2023-10-27 12:37 UTC (permalink / raw)
To: git
In-Reply-To: <cd248c66-fc71-4783-9195-02c8811744b8@app.fastmail.com>
On Fri, Oct 27, 2023 at 03:29:58PM +0300, Konstantin Khomoutov wrote:
[...]
> Also please check the core.editor configuration setting - it might be set,
> and then you should try to modify it instead.
[...]
This means Git's configuration setting.
You might run
git config --get-all --show-scope core.editor
in your shell to get the values of this setting printed, if set, prefixed with
the scope these values are defined in (see `git help config` or [1] for more
info).
1. https://www.git-scm.com/docs/git-config
^ permalink raw reply
* Re: [RESEND v2] git-rebase.txt: rewrite docu for fixup/squash (again)
From: Marc Branchaud @ 2023-10-27 12:39 UTC (permalink / raw)
To: Oswald Buddenhagen
Cc: git, Junio C Hamano, Phillip Wood, Christian Couder,
Charvi Mendiratta
In-Reply-To: <ZTg0zXkvSQ6L+4Oj@ugly>
On 2023-10-24 17:19, Oswald Buddenhagen wrote:
>
>>> +The commit message for the folded commit is the concatenation of the
>>> +message of the first commit with those of commits identified by
>>> "squash"
>>
>> s/message of the first commit/picked commit's message/
>>
> that does indeed sound better, but i think it's more confusing (and
> potentially even more so when translated directly). i guess one could
> use "pick'd commit's", but that's kind of ugly again.
Let the translators worry about how to phrase it in other languages. In
English "picked" is the right choice. You should not presume that other
languages will want to use the word "pick" verbatim.
M.
^ permalink raw reply
* Re: using oldest date when squashing commits
From: Oswald Buddenhagen @ 2023-10-27 12:45 UTC (permalink / raw)
To: Marc Branchaud
Cc: Johannes Sixt, Junio C Hamano, Phillip Wood, phillip.wood, git
In-Reply-To: <70b8d4d8-f4b5-4cd7-b73a-1d7393d84266@xiplink.com>
On Fri, Oct 27, 2023 at 08:34:40AM -0400, Marc Branchaud wrote:
>I never use "fixup -C" (or -c), but I do use squash/fixup a lot. I
>find that I would prefer it if Git used the most recent Author date
>from the set of commits being combined, rather than preserving the
>picked commit's Author date.
>
that would be unreliable, as plain amends wouldn't be reflected. that
may be rare in your workflow, but still.
>Sometimes it takes quite a while for me to get a piece of work sorted
>out, and I would rather have the Author date in the end-result commit
>reflect the work's completion time than its initiation time.
>
afaict, you need to get used to `--amend --reset-author` all commits
before you push to achieve this reliably. that can be easily automated
by using -x with rebase -i (filter-repo (ex filter-branch) would also
work).
regards
^ permalink raw reply
* Re: [PATCH v2 5/5] ci: add support for GitLab CI
From: Phillip Wood @ 2023-10-27 13:02 UTC (permalink / raw)
To: Patrick Steinhardt, phillip.wood; +Cc: git, Oswald Buddenhagen
In-Reply-To: <ZTulo_rh_G-lfICQ@tanuki>
On 27/10/2023 12:57, Patrick Steinhardt wrote:
> Hum. After having a look at `ci/run-docker-build.sh` I don't feel like
> it's sensible to update it. It's not even used anymore by our CI but
> only by `ci/run-docker.sh`, which seems to be more of a developer-facing
> script?
>
> As you said, this smells like rotting bits that might rather be removed.
> But in any case, as they don't relate to our current CI infrastructure
> except for being in "ci/" I'll leave them be for now.
I was trying to suggest that we start using these scripts again. The
fact that we run the dockerized tests as root on the other CI platforms
is a regression from what we used to do. I'm not an expert but I think
for the builds that use docker we're essentially using the some build
environment regardless of the CI provider so it would make sense to
handle them all in the same way. I think the existing script uses "su"
but we could change it to use "sudo" like you're doing here.
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH] send-email: move validation code below process_address_list
From: Junio C Hamano @ 2023-10-27 13:04 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Michael Strawbridge, Jeff King, Bagas Sanjaya, Git Mailing List
In-Reply-To: <20231025074317.r3sydthautjjsf5y@pengutronix.de>
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:
>> This fixes email address validation errors when the optional
>> perl module Email::Valid is installed and multiple addresses are passed
>> in on a single to/cc argument like --to=foo@example.com,bar@example.com.
>>
>> Reported-by: Bagas Sanjaya <bagasdotme@gmail.com>
>> Signed-off-by: Michael Strawbridge <michael.strawbridge@amd.com>
>
> If you do Fixes: trailers as the kernel does, this could get:
>
> Fixes: a8022c5f7b67 ("send-email: expose header information to git-send-email's sendemail-validate hook")
While referring to a concrete commit object name is great, we tend
not to use that particular trailer in this project; rather, we
prefer to see description on how and in what way the culprit change
was undesirable.
> I tested this patch on top of main (2e8e77cbac8a) and it fixes the
> regression I reported in a separate thread (where Jeff pointed out this
> patch as fixing it).
Great. Thanks.
^ permalink raw reply
* Re: [PATCH v3 0/5] config-parse: create config parsing library
From: Junio C Hamano @ 2023-10-27 13:04 UTC (permalink / raw)
To: Josh Steadmon; +Cc: Jonathan Tan, Taylor Blau, git, calvinwan, glencbz
In-Reply-To: <ZTluW0Ha2W4NYaJd@google.com>
Josh Steadmon <steadmon@google.com> writes:
> Thanks everyone for the revived discussion here. I think I agree, this
> series is not going in the right direction. Additionally, our internal
> use case for this change has evaporated, so let's just drop the series.
> We can pick it up again later if interest returns.
OK. Let's scrap it for now.
The "internal use case" behind a proposed feature changing so
quickly is a bit worrying. What is good for this project should
ideally be good for everybody, not only for satisfying a particular
$CORP needs of the day. But I think the idea of giving enhanced
visibility into stakeholder companies directions and priorities
Emily (I think?) floated during the contributors' summit may help
reduce such a risk, hopefully.
Thanks.
^ permalink raw reply
* Re: [PATCH 00/12] show-ref: introduce mode to check for ref existence
From: Phillip Wood @ 2023-10-27 13:06 UTC (permalink / raw)
To: Patrick Steinhardt, phillip.wood
Cc: Han-Wen Nienhuys, Junio C Hamano, git, Eric Sunshine
In-Reply-To: <ZTo2Aq2wAxx_6Laz@tanuki>
Hi Patrick
On 26/10/2023 10:48, Patrick Steinhardt wrote:
> On Wed, Oct 25, 2023 at 03:44:33PM +0100, Phillip Wood wrote:
>> On 25/10/2023 15:26, Han-Wen Nienhuys wrote:
>>> On Tue, Oct 24, 2023 at 9:17 PM Junio C Hamano <gitster@pobox.com> wrote:
>>>>
>>>> Patrick Steinhardt <ps@pks.im> writes:
>>>>
>>>>> this patch series introduces a new `--exists` mode to git-show-ref(1) to
>>>>> explicitly check for the existence of a reference, only.
>>>>
>>>> I agree that show-ref would be the best place for this feature (not
>>>> rev-parse, which is already a kitchen sink). After all, the command
>>>> was designed for validating refs in 358ddb62 (Add "git show-ref"
>>>> builtin command, 2006-09-15).
>>>>
>>>> Thanks. Hopefully I can take a look before I go offline.
>>>
>>> The series description doesn't say why users would care about this.
>>>
>>> If this is just to ease testing, I suggest adding functionality to a
>>> suitable test helper. Anything you add to git-show-ref is a publicly
>>> visible API that needs documentation and comes with a stability
>>> guarantee that is more expensive to maintain than test helper
>>> functionality.
>>
>> Does the new functionality provide a way for scripts to see if a branch is
>> unborn (i.e. has not commits yet)? I don't think we have a way to
>> distinguish between a ref that points to a missing object and an unborn
>> branch at the moment.
>
> You could do it with two commands:
>
> ```
> target=$(git symbolic-ref HEAD)
> git show-ref --exists "$target"
> case "$?" in
> 2)
> echo "unborn branch";;
> 0)
> echo "branch exists";;
> *)
> echo "could be anything, dunno";;
> esac
> ```
>
> While you could use git-rev-parse(1) instead of git-show-ref(1), you
> wouldn't be able to easily distinguish the case of a missing target
> reference and any other kind of error.
Thanks, it is helpful to have an example use outside of the test suite.
Best Wishes
Phillip
^ permalink raw reply
* Re: [RESEND v2] git-rebase.txt: rewrite docu for fixup/squash (again)
From: Oswald Buddenhagen @ 2023-10-27 13:08 UTC (permalink / raw)
To: Marc Branchaud
Cc: git, Junio C Hamano, Phillip Wood, Christian Couder,
Charvi Mendiratta
In-Reply-To: <b71d066b-104a-4c60-9319-b3c635be6efc@xiplink.com>
On Fri, Oct 27, 2023 at 08:39:03AM -0400, Marc Branchaud wrote:
>On 2023-10-24 17:19, Oswald Buddenhagen wrote:
>>>> +The commit message for the folded commit is the concatenation of
>>>> the
>>>> +message of the first commit with those of commits identified by
>>>> "squash"
>>>
>>> s/message of the first commit/picked commit's message/
>>>
>> that does indeed sound better, but i think it's more confusing (and
>> potentially even more so when translated directly). i guess one could
>> use "pick'd commit's", but that's kind of ugly again.
>
>Let the translators worry about how to phrase it in other languages.
>
my experience tells me that this isn't a good idea. translations are
often done by people who have little domain knowledge of what they
translate. it's a good idea to guide them.
also, the english text is often read by people who barely understand
english, and will attempt literal translations in their head.
>In English "picked" is the right choice.
>
the squashed commits also fit the natural use of "picked", because
"picking" means "selecting". it's not advisable to use this potentially
ambiguous term when there is an unambiguous alternative way to identify
the commit available.
>You should not presume that other languages will want to use the word
>"pick" verbatim.
>
well, i actually should, because it's the command's own name, which
definitely shouldn't be translated.
regards
^ permalink raw reply
* Re: [PATCH v3] git-rebase.txt: rewrite docu for fixup/squash (again)
From: Marc Branchaud @ 2023-10-27 13:14 UTC (permalink / raw)
To: Oswald Buddenhagen, git
Cc: Junio C Hamano, Phillip Wood, Taylor Blau, Christian Couder,
Charvi Mendiratta
In-Reply-To: <20231025102932.1202299-1-oswald.buddenhagen@gmx.de>
On 2023-10-25 06:29, Oswald Buddenhagen wrote:
> Create a clear top-down structure which makes it hopefully unambiguous
> what happens when.
>
> The behavior in the presence of multiple "fixup -c" is somewhat
> questionable, as arguably it would be better to complain about it rather
> than letting the last instance win. But for the time being we document
> the status quo, with a note that it is not guaranteed. Note that
> actually changing it would require --autosquash eliding the superseded
> uses.
I do not think this kind of editorializing belongs in the commit's
message, but this likely isn't the first commit message that expresses
an opinion.
> Also emphasize that the author info of the first commit is preserved
> even in the presence of "fixup -c", as this diverges from "git commit
> -c"'s behavior. New options matching the latter should be introduced for
> completeness.
>
> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
>
> ---
> v3:
> - adjust to reality, and elaborate in the commit message why it's
> arguably somewhat suboptimal
>
> i deliberated the 'command "pick"' word order swap suggested by marc,
> but while it improves things locally, it somehow doesn't flow with the
> "redundancy-reduced" last part of the sentence.
>
> v2:
> - slight adjustments inspired by marc. however, i left most things
> unchanged or even went in the opposite direction, because i assume the
> readers to be sufficiently context-sensitive, and the objective is
> merely to be not actively confusing. adding redundancy in the name of
> clarity would just make the text stylistically inferior and arguably
> harder to read.
>
> Cc: Junio C Hamano <gitster@pobox.com>
> Cc: Phillip Wood <phillip.wood123@gmail.com>
> Cc: Taylor Blau <me@ttaylorr.com>
> Cc: Christian Couder <christian.couder@gmail.com>
> Cc: Charvi Mendiratta <charvi077@gmail.com>
> Cc: Marc Branchaud <marcnarc@xiplink.com>
> ---
> Documentation/git-rebase.txt | 30 ++++++++++++++++--------------
> 1 file changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index e7b39ad244..578d1d34a6 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -890,20 +890,22 @@ 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
> +message of the first commit with those of commits identified by "squash"
> +commands, omitting those 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 (if multiple are present, the last one
> +takes precedence, but this should not be relied upon).
I like the overall phrasing here.
But I think you should remove the "but this should not be relied upon"
phrase. This reads as if Git's current behaviour is undefined, which
most definitely is not true.
Even changing this to something like "but this might change in the
future" is unhelpful. Everything in Git is subject to change over a
long-enough time span, so the same could be said about every aspect of Git.
Until the behaviour actually changes, it's perfectly fine for people to
use multiple "fixup -c" commands. There's no reason to scare them off
of it.
> +If the resulting commit message is a concatenation of multiple messages,
> +an editor is opened allowing you to edit it. This is also the case for a
> +message obtained via "fixup -c", while using "fixup -C" instead skips
> +the editor; this is analogous to the behavior of `git commit`.
> +The author information (including date/timestamp) always comes from
> +the first commit; this is the case even if "fixup -c/-C" is used,
> +contrary to what `git commit` does.
This phrasing is much better.
Thanks for putting up with my pedantry!
M.
^ permalink raw reply
* Re: [PATCH v2 5/5] ci: add support for GitLab CI
From: Phillip Wood @ 2023-10-27 13:17 UTC (permalink / raw)
To: Oswald Buddenhagen, Patrick Steinhardt; +Cc: git
In-Reply-To: <ZTuYjzOfG0y27Sho@ugly>
On 27/10/2023 12:01, Oswald Buddenhagen wrote:
> On Fri, Oct 27, 2023 at 11:25:41AM +0200, Patrick Steinhardt wrote:
>> + export GIT_PROVE_OPTS="--timer --jobs $(nproc)"
>> + export GIT_TEST_OPTS="--verbose-log -x"
>>
> fwiw (as this is again only copied), export with assignment is a
> bash-ism
Not according to
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#export
SYNOPSIS
export name[=word]...
DESCRIPTION
The shell shall give the export attribute to the variables
corresponding to the specified names, which shall cause them
to be in the environment of subsequently executed commands. If
the name of a variable is followed by = word, then the value
of that variable shall be set to word.
It is true that in our test suite we separate a variable assignment when
exporting. Presumably that is because someone reported that their shell
did not support the "export name=WORD" syntax in the past. As we're
already using this syntax with the same docker images in Github Actions
I think we can assume it is safe here.
Best Wishes
Phillip
^ permalink raw reply
* Re: using oldest date when squashing commits
From: Marc Branchaud @ 2023-10-27 13:20 UTC (permalink / raw)
To: Oswald Buddenhagen
Cc: Johannes Sixt, Junio C Hamano, Phillip Wood, phillip.wood, git
In-Reply-To: <ZTuw7ziOnTunMmML@ugly>
On 2023-10-27 08:45, Oswald Buddenhagen wrote:
> On Fri, Oct 27, 2023 at 08:34:40AM -0400, Marc Branchaud wrote:
>> I never use "fixup -C" (or -c), but I do use squash/fixup a lot. I
>> find that I would prefer it if Git used the most recent Author date
>> from the set of commits being combined, rather than preserving the
>> picked commit's Author date.
>>
> that would be unreliable, as plain amends wouldn't be reflected. that
> may be rare in your workflow, but still.
I'm not talking about amends, plain or otherwise. I'm talking about
fixup/squash.
(Why do you focus so much an making rebase and commit behave
identically? There is no reason to do so just because they happen to
share a couple of parameter names.)
>> Sometimes it takes quite a while for me to get a piece of work sorted
>> out, and I would rather have the Author date in the end-result commit
>> reflect the work's completion time than its initiation time.
>>
> afaict, you need to get used to `--amend --reset-author` all commits
> before you push to achieve this reliably. that can be easily automated
> by using -x with rebase -i (filter-repo (ex filter-branch) would also
> work).
Yes, I know how to force my desired author date on commits, thanks.
M.
^ permalink raw reply
* Re: using oldest date when squashing commits
From: Oswald Buddenhagen @ 2023-10-27 13:26 UTC (permalink / raw)
To: Marc Branchaud
Cc: Johannes Sixt, Junio C Hamano, Phillip Wood, phillip.wood, git
In-Reply-To: <6d100655-ffd4-4282-87b5-cfdd101dba63@xiplink.com>
On Fri, Oct 27, 2023 at 09:20:04AM -0400, Marc Branchaud wrote:
>
>On 2023-10-27 08:45, Oswald Buddenhagen wrote:
>> On Fri, Oct 27, 2023 at 08:34:40AM -0400, Marc Branchaud wrote:
>>> I never use "fixup -C" (or -c), but I do use squash/fixup a lot. I
>>> find that I would prefer it if Git used the most recent Author date
>>> from the set of commits being combined, rather than preserving the
>>> picked commit's Author date.
>>>
>> that would be unreliable, as plain amends wouldn't be reflected. that
>> may be rare in your workflow, but still.
>
>I'm not talking about amends, plain or otherwise.
>
but why wouldn't you? your use case of marking the date of completion
naturally covers all ways of amending commits, whether directly or via
squashing.
regards
^ permalink raw reply
* Re: [RFC PATCH v2 0/6] Noobify format for status, add, restore
From: Dragan Simic @ 2023-10-27 13:32 UTC (permalink / raw)
To: Jacob Stopak; +Cc: git, Junio C Hamano, Oswald Buddenhagen
In-Reply-To: <20231026224615.675172-1-jacob@initialcommit.io>
On 2023-10-27 00:46, Jacob Stopak wrote:
> Take into account reviewer feedback by doing several things
> differently:
>
> * Rename this feature (for now) as "noob format mode" (or just "noob
> mode") instead of the original "--table" verbiage. As pointed out,
> this no longer ties the name of the setting to it's proposed
> implementation detail as a table. Noob mode is not necessarily the
> right name, just a placeholder for now. Unless people like it :D
>
> * Instead of manually having to invoke the -t, --table every time
> this
> format is to be used, set the config option "status.noob" to true.
> Although this is logically tied to the status command, there are
> many
> commands that produce status output, (and this series adds more),
> so
> assume that if the user wants to see the status this way, that it
> should be enabled whenever the status info is displayed.
How would "status.noob" relate to and coexist with possible future
configuration options named "<command>.verbose", which would be somewhat
similar to the currently existing "commit.verbose" option? IOW, perhaps
it would be better to have per-command options "<command>.verbose =
noob" or, even better, "<command>.verbose = extended", to make it all
more future-proof and more granular.
> * When running "git add" and "git restore" while noob mode is
> enabled,
> perform the add/restore function as usual, but display the table
> formatted output with arrows showing how file changes moved around.
> Displaying the output in this understandable format after each
> command execution allows the noob to immediately see what they did.
> Although this series only implements for status, add, and restore,
> this output format would make sense in other commands like rm, mv,
> commit, clean, and stash.
>
> * Works consistently with commands that already have a --dry-run
> (-n) option. The dry run shows the exact same output, but
> doesn't actually do the thing.
>
> * If `advice.statusHints` is true, add a table footer with status
> hints.
> Shorten these hints so that they are still clear but better fit
> into a
> table. Make the hint text yellow to distinguish them. The hints
> only
> appear when explicitly running "git status", which helps the user
> answer the question "what can I do next?". Hints are omitted in
> "impact" commands like add and restore. Having hints here distracts
> from the file change moves being showed in the table by arrows.
>
> TODO:
>
> * "git status" outputs myriad other information depending on the
> state
> of the repo, like branch info, merge conflicts, rebase info,
> bisect,
> etc. Need to think about how to convey that info with the new
> setting.
>
> * Some commands (like stash) might need more than 3 table columns to
> display everything clearly.
>
> * For destructive commands, think about adding a prompt describing
> the
> effect, so the user can confirm before the action is taken.
>
> * Fix horrible things in the patch series code.
>
> * Probably other things.
>
> Play around with it! It's fun!
>
> Jacob Stopak (6):
> status: add noob format from status.noob config
> status: handle long paths in noob format
> add: implement noob mode
> add: set unique color for noob mode arrows
> restore: implement noob mode
> status: add advice status hints as table footer
>
> Makefile | 2 +
> builtin/add.c | 47 +++++--
> builtin/checkout.c | 46 +++++--
> builtin/commit.c | 157 +----------------------
> commit.c | 2 +
> noob.c | 198 +++++++++++++++++++++++++++++
> noob.h | 21 ++++
> read-cache-ll.h | 10 +-
> read-cache.c | 41 +++++-
> table.c | 301 +++++++++++++++++++++++++++++++++++++++++++++
> table.h | 6 +
> wt-status.c | 75 +++++++----
> wt-status.h | 6 +
> 13 files changed, 708 insertions(+), 204 deletions(-)
> create mode 100644 noob.c
> create mode 100644 noob.h
> create mode 100644 table.c
> create mode 100644 table.h
^ permalink raw reply
* Re: using oldest date when squashing commits
From: Marc Branchaud @ 2023-10-27 13:46 UTC (permalink / raw)
To: Oswald Buddenhagen
Cc: Johannes Sixt, Junio C Hamano, Phillip Wood, phillip.wood, git
In-Reply-To: <ZTu6cqUec3L2PpUC@ugly>
On 2023-10-27 09:26, Oswald Buddenhagen wrote:
> On Fri, Oct 27, 2023 at 09:20:04AM -0400, Marc Branchaud wrote:
>>
>> On 2023-10-27 08:45, Oswald Buddenhagen wrote:
>>> On Fri, Oct 27, 2023 at 08:34:40AM -0400, Marc Branchaud wrote:
>>>> I never use "fixup -C" (or -c), but I do use squash/fixup a lot. I
>>>> find that I would prefer it if Git used the most recent Author date
>>>> from the set of commits being combined, rather than preserving the
>>>> picked commit's Author date.
>>>>
>>> that would be unreliable, as plain amends wouldn't be reflected. that
>>> may be rare in your workflow, but still.
>>
>> I'm not talking about amends, plain or otherwise.
>>
> but why wouldn't you? your use case of marking the date of completion
> naturally covers all ways of amending commits, whether directly or via
> squashing.
Please do not presume what my use cases might be. I'm quite happy with
commit's behaviour, but not happy with rebase's fixup/squash behaviour
because it's too much work to achieve the desired results. (Results
which, as I said, I don't care about enough to bother changing anyway).
M.
^ permalink raw reply
* Re: [PATCH 5/5] ci: add support for GitLab CI
From: Phillip Wood @ 2023-10-27 14:32 UTC (permalink / raw)
To: Oswald Buddenhagen; +Cc: Patrick Steinhardt, git
In-Reply-To: <ZTuUa5bA8dh29cyv@ugly>
On 27/10/2023 11:43, Oswald Buddenhagen wrote:
> On Fri, Oct 27, 2023 at 11:22:35AM +0100, Phillip Wood wrote:
>> On 27/10/2023 09:17, Patrick Steinhardt wrote:
>>>>> + CI_BRANCH="$CI_COMMIT_REF_NAME"
>>>>> + CI_COMMIT="$CI_COMMIT_SHA"
>>>>>
>>>> assignments need no quoting to prevent word splitting.
>>>> repeats below.
>>>>
>>>>> + case "$CI_JOB_IMAGE" in
>>>>>
>>>> ... as does the selector in case statements.
>>>
>>> True, but I'm simply matching the coding style in this script.
>>
>> I think it is quite common for us to quote variables when it isn't
>> strictly necessary as it makes it clear to anyone reading the script
>> that there is no word splitting going on
>
>> and ensures that we don't start splitting the variable if the contents
>> changes in the future.
>>
> the point was that it *isn't* content-dependent; it's simply the shell
> rules.
Oh, I'd misunderstood what you were saying which was that assignment and
case statements are not subject to field splitting.
> of course, many people (apparently you included) don't know these
> subtleties
I find this comment to be condescending, needlessly antagonistic and
completely at odds with the norms of constructive discussion on this list.
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH v3] bugreport: reject positional arguments
From: Phillip Wood @ 2023-10-27 14:41 UTC (permalink / raw)
To: Eric Sunshine, emilyshaffer; +Cc: git, Emily Shaffer, Sheik, Dragan Simic
In-Reply-To: <CAPig+cTmYtWR=QN3LeN9yw3HmsKEmD2fUiRjKf=eJHhAZyT-yA@mail.gmail.com>
On 26/10/2023 18:18, Eric Sunshine wrote:
> On Thu, Oct 26, 2023 at 11:55 AM <emilyshaffer@google.com> wrote:
>> git-bugreport already rejected unrecognized flag arguments, like
>> `--diaggnose`, but this doesn't help if the user's mistake was to forget
>> the `--` in front of the argument. This can result in a user's intended
>> argument not being parsed with no indication to the user that something
>> went wrong. Since git-bugreport presently doesn't take any positionals
>> at all, let's reject all positionals and give the user a usage hint.
>>
>> Signed-off-by: Emily Shaffer <nasamuffin@google.com>
>> ---
>> Per Eric's and Dragan's comments, dropped the null checking for argv[0].
>> No point in being too paranoid, I suppose :)
>>
>> Note that after this morning it's not likely that I'll be able to find
>> time to update this again so quickly, so if there are other nits,
>> reviewers can feel free to send their own rerolls rather than waiting
>> for me to see it and turn the patch around.
>
> Thanks. This version looks good enough to me. Just one minor comment below...
>
>> diff --git a/t/t0091-bugreport.sh b/t/t0091-bugreport.sh
>> @@ -69,6 +69,13 @@ test_expect_success 'incorrect arguments abort with usage' '
>> +test_expect_success 'incorrect positional arguments abort with usage and hint' '
>> + test_must_fail git bugreport false 2>output &&
>> + test_i18ngrep usage output &&
>> + test_i18ngrep false output &&
>> + test_path_is_missing git-bugreport-*
>> +'
>
> I didn't really pay attention to the test in earlier rounds so didn't
> notice this, but these days we just use 'grep' rather than
> 'test_i18ngrep'. (Indeed, the existing tests in this script use
> 'grep'.)
It is rather unfortunate that test_i18ngrep was deprecated without
providing an alternative that offers the same debugging experience. When
test_i18ngrep fails it prints a message with the pattern and text that
failed to match so it is easy to see where the test failed. If grep
fails there is no output and so unless the test is run with "-x" it can
be hard to see which command caused the test to fail.
Best Wishes
Phillip
^ permalink raw reply
* Re: Constant recompilation when GENERATE_COMPILATION_DATABASE is set
From: Philippe Blain @ 2023-10-27 15:41 UTC (permalink / raw)
To: brian m. carlson, git
In-Reply-To: <ZTui2NcJ3Ax_PIGO@tapette.crustytoothpaste.net>
Hi brian,
Le 2023-10-27 à 07:45, brian m. carlson a écrit :
> I typically use clangd with Git for the nice language server protocol
> (LSP) support. I noticed that when GENERATE_COMPILATION_DATABASE is set
> (which is used so clangd can find the proper flags), `make` rebuilds all
> of the files every time, even if I do it back to back. This persists
> even after a `git clean -dxf`.
>
> Obviously, this is not great, since it means the turnaround time to
> compile changes is slower. Unfortunately, I'm not a huge expert in make
> and I'm unsure what the right solution is here. I'd appreciate anyone's
> thoughts on how to improve this.
>
> This is on a Debian sid (unstable) amd64 system with the following
> configuration. The version of clang in use is Debian's 16.0.6.
>
> ----
> NO_OPENSSL=1
> DC_SHA1=1
> NETTLE_SHA256=1
> DEVELOPER=1
> NO_SVN_TESTS=1
> NO_PYTHON=1
> USE_ASCIIDOCTOR=1
> USE_ASCIIDOCTOR_MANPAGE=1
> CC=clang
> GENERATE_COMPILATION_DATABASE=yes
> CSPRNG_METHOD=getrandom getentropy
> ----
>
Indeed, with GENERATE_COMPILATION_DATABASE=yes and CC=clang I can reproduce with:
make clean
make -j
make -j # this rebuilds everything
make -j # this does not rebuild anything
so the second 'make' invocation is buggy, but not subsequent ones.
I took a look at the 'make -d' output and we can see it is the dependency on
the 'compile_commands' directory that is the culprit, on the second invocation
it is newer than the the first object file that makes checks.
This makes sense since each time we compile an object we put the JSON fragment in
compile_commands, so the directory is updated each time an object is built, so
clearly its modification date will be newer than the one of the first object built.
One way to fix this is to use the same trick that is used for the '.depend' directories
created to hold the Makefile dependencies:
diff --git a/Makefile b/Makefile
index 5776309365..f6f7255dd1 100644
--- a/Makefile
+++ b/Makefile
@@ -2701,7 +2701,7 @@ endif
compdb_dir = compile_commands
ifeq ($(GENERATE_COMPILATION_DATABASE),yes)
-missing_compdb_dir = $(compdb_dir)
+missing_compdb_dir = $(filter-out $(wildcard $(compdb_dir)), $(compdb_dir))
$(missing_compdb_dir):
@mkdir -p $@
That is, we define missing_compdb_dir to 'compile_commands' only if the directory
is not yet created.
I'll try to send a proper patch for this.
Thanks for the report,
Philippe.
^ permalink raw reply related
* Re: Regression: git send-email fails with "Use of uninitialized value $address" + "unable to extract a valid address"
From: Junio C Hamano @ 2023-10-27 15:44 UTC (permalink / raw)
To: Todd Zullinger
Cc: Michael Strawbridge, Jeff King, Uwe Kleine-König,
Luben Tuikov, git, entwicklung
In-Reply-To: <ZTp7xvXDw1GF-NUB@pobox.com>
Todd Zullinger <tmz@pobox.com> writes:
> Alternately, perhaps having Email::Valid as an optional
> dependency is worth reconsidering. If it's truly important
> to validation, make it a requirement. If it's not, then
> drop it to simplify the code and avoid these sort of issues.
Reducing the possible "valid" configurations we support is a very
tempting proposition.
> If I make the git package require it to ensure consistent
> behavior then some folks will -quite rightly- complain that
> it should not be a requirement. If I keep it an optional
> dependency, then debugging becomes more difficult for the
> reasons we've seen in these recent (and not-so-recent)
> threads.
Very true.
> I'd lean toward dropping the dependency entirely and leave
> the more basic validation of git-send-email in place. That
> may not catch every type of address error, but I would argue
> that what we do without Email::Valid is perfectly reasonable
> for checking basic email address syntax sanity.
Yes, it is very tempting, and given that we have to keep our
fallback codepath working for those without Email::Valid ANYWAY,
as long as the dependency is merely optional, I very much agree
with your argument here.
> On a related note, one issue¹ we had reported in Fedora
> after making Email::Valid a requirement was that it rejected
> messages where the local part was too long, per the relevant
> RFC's. But these were generated addresses from GitLab. The
> addresses worked in practice. While Email::Valid was
> technically correct in rejecting such addresses, it didn't
> improve the experience of git send-email users.
I am of two minds here. I can sympathize with both positions.
- Trying to be strict to what we send out to the world by using
Email::Valid that tries to be more RFC kosher matches "be strict
in what you send out, be lenient in what you receive" mantra
- Rejecting what works in practice and in real world tend to help
users.
If we require Email::Valid, then sriking the balance between the
above two will entirely become the responsibility of them; any
end-user who complains "the validation is overly strict" will get
"talk to authors of Email::Valid".
If we ditch Email::Valid, it will become _our_ responsibility, which
means a bit of extra maintenance burden to this project. But perhaps
it is worth it? I dunno.
Having Email::Valid as an optional dependency does not place us in a
position better than either of these two, so from that point of view,
too, I like your "we should either make it required or unused, not
an optional dependency" very much.
Thanks.
^ permalink raw reply
* Re: [PATCH v2 5/5] ci: add support for GitLab CI
From: Oswald Buddenhagen @ 2023-10-27 15:53 UTC (permalink / raw)
To: phillip.wood; +Cc: Patrick Steinhardt, git
In-Reply-To: <d00b02e9-fb05-44bc-90ee-1851ef98dd26@gmail.com>
On Fri, Oct 27, 2023 at 02:17:02PM +0100, Phillip Wood wrote:
>On 27/10/2023 12:01, Oswald Buddenhagen wrote:
>> On Fri, Oct 27, 2023 at 11:25:41AM +0200, Patrick Steinhardt wrote:
>>> + export GIT_PROVE_OPTS="--timer --jobs $(nproc)"
>>> + export GIT_TEST_OPTS="--verbose-log -x"
>>>
>> fwiw (as this is again only copied), export with assignment is a
>> bash-ism
>
>Not according to
>https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#export
>
hmm, it's there since at least SUSv1, aka XPG4v2, in 1994. i didn't
bother digging deeper.
>It is true that in our test suite we separate a variable assignment
>when exporting. Presumably that is because someone reported that their
>shell did not support the "export name=WORD" syntax in the past.
>
most likely it's just a historical default, not the result of a specific
bug report. the assumption that it's bash-specific is wide-spread.
>As we're already using this syntax with the same docker images in
>Github Actions I think we can assume it is safe here.
>
i guess so. docker seems a tad unlikely to run some ancient bourne
shells ...
regards
^ permalink raw reply
* Re: [PATCH v1 3/4] config: factor out global config file retrievalync-mailbox>
From: Junio C Hamano @ 2023-10-27 15:54 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Kristoffer Haugsbakk, Taylor Blau, git, stolee
In-Reply-To: <ZTjMMC1GiPJUXnQm@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
> This parameter would only exist for the purpose of the error message,
> right? If so, I think that'd be overkill. If we want to have differing
> errors depending on how the function is called the best way to handle
> that would likely be to generate the error message at the callsite
> instead of in the library itself.
We would need to make sure the lower-level helpers need to be able
to tell what kind of failure they saw (in other words, why they are
failing) to the callers, which may require a bit of designing the
error return convention and plumbing through necessary pieces of
information, but the longer term payoff would be great.
I do not think this is such a case, but if the lower-level needs to
fail differently (e.g., the thing not existing is acceptable when
writing as we will create a new one, but is a fail-worthy error when
reading), then the caller needs to give that down the callchain,
though.
^ permalink raw reply
* Re: [PATCH v3] git-rebase.txt: rewrite docu for fixup/squash (again)
From: Oswald Buddenhagen @ 2023-10-27 16:12 UTC (permalink / raw)
To: Marc Branchaud
Cc: git, Junio C Hamano, Phillip Wood, Taylor Blau, Christian Couder,
Charvi Mendiratta
In-Reply-To: <56e3e974-a027-439f-871d-c7fbae65a04e@xiplink.com>
On Fri, Oct 27, 2023 at 09:14:42AM -0400, Marc Branchaud wrote:
>On 2023-10-25 06:29, Oswald Buddenhagen wrote:
>> The behavior in the presence of multiple "fixup -c" is somewhat
>> questionable, as arguably it would be better to complain about it rather
>> than letting the last instance win. But for the time being we document
>> the status quo, with a note that it is not guaranteed. Note that
>> actually changing it would require --autosquash eliding the superseded
>> uses.
>
>I do not think this kind of editorializing belongs in the commit's
>message, but this likely isn't the first commit message that expresses
>an opinion.
>
commmit messages should elaborate alternatives considered, which
includes ones which depend on changes that can be reasonably expected to
possibly happen at some point.
>But I think you should remove the "but this should not be relied upon"
>phrase. This reads as if Git's current behaviour is undefined, which
>most definitely is not true.
>
>Even changing this to something like "but this might change in the
>future" is unhelpful. Everything in Git is subject to change over a
>long-enough time span, so the same could be said about every aspect of Git.
>
>Until the behaviour actually changes, it's perfectly fine for people to
>use multiple "fixup -c" commands. There's no reason to scare them off
>of it.
>
things can't change overnight; the resistance even the most trivial
behavior changes meet is enormous. so explicitly documenting long in
advance that something is subject to change is basically the only way to
get it changed at all.
specifically for this feature, there is no reason at all to rely on this
behavior when hand-editing the todo list, and occurrences most likely
indicate a mistake, which is why i would prefer it to be rejected.
regards
^ permalink raw reply
* Re: [RFC PATCH v2 0/6] Noobify format for status, add, restore
From: Jacob Stopak @ 2023-10-27 17:13 UTC (permalink / raw)
To: Dragan Simic; +Cc: git, Junio C Hamano, Oswald Buddenhagen
In-Reply-To: <ca47d328c280e4b4c13bfa6dd9958a57@manjaro.org>
On Fri, Oct 27, 2023 at 03:32:40PM +0200, Dragan Simic wrote:
> On 2023-10-27 00:46, Jacob Stopak wrote:
> > Take into account reviewer feedback by doing several things differently:
> >
> > * Rename this feature (for now) as "noob format mode" (or just "noob
> > mode") instead of the original "--table" verbiage. As pointed out,
> > this no longer ties the name of the setting to it's proposed
> > implementation detail as a table. Noob mode is not necessarily the
> > right name, just a placeholder for now. Unless people like it :D
> >
> > * Instead of manually having to invoke the -t, --table every time this
> > format is to be used, set the config option "status.noob" to true.
> > Although this is logically tied to the status command, there are
> > many
> > commands that produce status output, (and this series adds more), so
> > assume that if the user wants to see the status this way, that it
> > should be enabled whenever the status info is displayed.
>
> How would "status.noob" relate to and coexist with possible future
> configuration options named "<command>.verbose", which would be somewhat
> similar to the currently existing "commit.verbose" option? IOW, perhaps it
> would be better to have per-command options "<command>.verbose = noob" or,
> even better, "<command>.verbose = extended", to make it all more
> future-proof and more granular.
Hmm, do there currently exist other <command>.verbose config settings
besides for commit? From what I can tell from "git help config", the
commit.verbose setting is the only one I see, and it just adds the diff
info into the editor if the user runs git commit without the -m flag, but
otherwise there seems to be no extra verbosity outputted.
I noticed that git add and git mv have "verbose" (-v, --verbose) cli flags
which just output the name of the file being added or renamed, and that
certain other commands like git branch has a verbose output which includes
the branch head commit hash and message in the output, so I guess this one
is actually kindof verbose in that it outputs more than the non-empty
default output.
So it seems like currently "verbose" is used for various things among the
command set, sometimes meaning "add something into the template if one is
used" or "add some tiny output to a command that has no default output"
(which still seems more "--shy" than "--verbose" :P) or "add some
additional output to a command that already has some sparse output".
Another thing is that commands like status have multiple flags that can be
used to specify the output format, such as --short, --long, --porcelain,
etc, but only --short seems to be configurable as a git config setting.
Is there a reason (besides backward compatibility I guess) that these
aren't rolled into a single thing like --format=<type>? This seems like
it would be the easiest way to future proof for new formats like
--format=verbose, --format=noob, --format=extended, etc.
From a noob's perspective though, does adding a config setting for each
command really make sense? I'm kindof envisioning this setting now as a
"mode" that is either enabled for all commands it affects or for none.
And it's highly unlikely a newish user would individually discover which
commands this "extended" format is available for, and run "git config
<command>.verbose = extended" for every one. I mean we could do that
in case there are folks who only want it for specific commands, but to
fulfill it's purpose I think there should definetely be some general way
to enable the setting for all commands that have it.
^ permalink raw reply
* Re: [PATCH 5/5] ci: add support for GitLab CI
From: Oswald Buddenhagen @ 2023-10-27 17:47 UTC (permalink / raw)
To: Phillip Wood; +Cc: Patrick Steinhardt, git
In-Reply-To: <b6ef74b7-2c77-4621-9969-d911c34561d5@crinan.ddns.net>
On Fri, Oct 27, 2023 at 03:32:48PM +0100, Phillip Wood wrote:
>On 27/10/2023 11:43, Oswald Buddenhagen wrote:
>> On Fri, Oct 27, 2023 at 11:22:35AM +0100, Phillip Wood wrote:
>>>>>> + CI_BRANCH="$CI_COMMIT_REF_NAME"
>>>>>> + CI_COMMIT="$CI_COMMIT_SHA"
>>>>>>
>>>>> assignments need no quoting to prevent word splitting.
>>>>> repeats below.
>>>>>
>>> I think it is quite common for us to quote variables when it isn't
>>> strictly necessary as it makes it clear to anyone reading the script
>>> that there is no word splitting going on
>>
>>> and ensures that we don't start splitting the variable if the contents
>>> changes in the future.
>>>
>> the point was that it *isn't* content-dependent; it's simply the shell
>> rules.
>
>Oh, I'd misunderstood what you were saying which was that assignment and
>case statements are not subject to field splitting.
>
>> of course, many people (apparently you included) don't know these
>> subtleties
>
>I find this comment to be condescending, needlessly antagonistic and
>completely at odds with the norms of constructive discussion on this list.
>
the observation was necessary for the point i subsequently made (which
was basically agreeing with the first part of your response).
i think it's a rather uncontroversial statement that many people don't
know the ins and outs of the shell's word splitting rules. in fact, this
is to be expected given how arbitrary the rules seem.
that you seem to be among these people was a reasonable inference from
what you actually wrote. mentioning it was meant to provide a segue and
add emphasis.
regards
^ 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