* Re: [PATCH v2 2/7] t0410: convert tests to use DEFAULT_REPO_FORMAT prereq
From: Junio C Hamano @ 2024-02-15 18:19 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Karthik Nayak
In-Reply-To: <2dd87f3126cedd39cb5b113053c90ee35ae0e5ff.1707985173.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> We have recently introduced a new DEFAULT_REPO_FORMAT prerequisite.
> Despite capturing the intent more directly, it also has the added
> benefit that it can easily be extended in the future in case we add new
> repository extensions. Adapt the tests to use it.
That is a good explanation. Instead of lifting the prerequisite and
forcing version 0 altogether, we can move by one step (I am puzzled
by "despite" though).
As I said in a separate message, as long as we make sure somebody
will test with version 0, we do not mind if other people test with
higher versions, so in that sense, the patch in v1 may be closer to
what we want to have in the longer term (but we need to figure out
how we "make sure somebody will test with version 0" first).
Thanks, will queue.
> -test_expect_success SHA1,REFFILES 'convert to partial clone with noop extension' '
> +test_expect_success DEFAULT_REPO_FORMAT 'convert to partial clone with noop extension' '
> rm -fr server client &&
> test_create_repo server &&
> test_commit -C server my_commit 1 &&
> @@ -60,7 +60,7 @@ test_expect_success SHA1,REFFILES 'convert to partial clone with noop extension'
> git -C client fetch --unshallow --filter="blob:none"
> '
>
> -test_expect_success SHA1,REFFILES 'converting to partial clone fails with unrecognized extension' '
> +test_expect_success DEFAULT_REPO_FORMAT 'converting to partial clone fails with unrecognized extension' '
> rm -fr server client &&
> test_create_repo server &&
> test_commit -C server my_commit 1 &&
^ permalink raw reply
* [PATCH] branch: rework the descriptions of rename and copy operations
From: Dragan Simic @ 2024-02-15 18:42 UTC (permalink / raw)
To: git
Move the descriptions of the <oldbranch> and <newbranch> arguments to the
descriptions of the branch rename and copy operations, where they naturally
belong. Also, improve the descriptions of these two branch operations and,
for completeness, describe the outcomes of forced operations.
Describing the arguments together with their respective operations, instead
of describing them separately in a rather unfortunate attempt to squeeze more
meaning out of fewer words, flows much better and makes the git-branch(1)
man page significantly more usable.
The subsequent improvements shall continue this approach by either dissolving
as many sentences from the "Description" section into the "Options" section,
or by having those sentences converted into some kind of more readable and
better flowing prose, as already discussed and outlined. [1][2]
[1] https://lore.kernel.org/git/xmqqttmmlahf.fsf@gitster.g/T/#u
[2] https://lore.kernel.org/git/xmqq8r4zln08.fsf@gitster.g/T/#u
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
---
Notes:
This patch was originally named "branch: clarify <oldbranch> and <newbranch>
terms further", submitted and discussed in another thread, [3] but the nature
of the patch has changed, causing the patch subject to be adjusted to match.
Consequently, this is effectively version 2 of the patch, which includes
detailed feedback from Kyle and Junio, who suggested moving/adding the
argument descriptions to their respective commands. This resulted in more
significant changes to the contents of the git-branch(1) man page, in an
attempt to make it more readable.
[3] https://lore.kernel.org/git/e2eb777bca8ffeec42bdd684837d28dd52cfc9c3.1707136999.git.dsimic@manjaro.org/T/#u
Documentation/git-branch.txt | 44 +++++++++++++++---------------------
1 file changed, 18 insertions(+), 26 deletions(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 0b0844293235..370ea43c0380 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -72,16 +72,6 @@ the remote-tracking branch. This behavior may be changed via the global
overridden by using the `--track` and `--no-track` options, and
changed later using `git branch --set-upstream-to`.
-With a `-m` or `-M` option, <oldbranch> will be renamed to <newbranch>.
-If <oldbranch> had a corresponding reflog, it is renamed to match
-<newbranch>, and a reflog entry is created to remember the branch
-renaming. If <newbranch> exists, -M must be used to force the rename
-to happen.
-
-The `-c` and `-C` options have the exact same semantics as `-m` and
-`-M`, except instead of the branch being renamed, it will be copied to a
-new name, along with its config and reflog.
-
With a `-d` or `-D` option, `<branchname>` will be deleted. You may
specify more than one branch for deletion. If the branch currently
has a reflog then the reflog will also be deleted.
@@ -128,18 +118,28 @@ Note that 'git branch -f <branchname> [<start-point>]', even with '-f',
refuses to change an existing branch `<branchname>` that is checked out
in another worktree linked to the same repository.
--m::
---move::
- Move/rename a branch, together with its config and reflog.
+-m [<oldbranch>] <newbranch>::
+--move [<oldbranch>] <newbranch>::
+ Rename an existing branch <oldbranch>, which if not specified defaults
+ to the current branch, to <newbranch>. The configuration variables
+ for the <oldbranch> branch and its reflog are also renamed appropriately
+ to be used with <newbranch>. Renaming fails if branch <newbranch>
+ already exists, but you can use `-M` or `--move --force` to overwrite
+ the files in existing branch <newbranch> while renaming.
--M::
+-M [<oldbranch>] <newbranch>::
Shortcut for `--move --force`.
--c::
---copy::
- Copy a branch, together with its config and reflog.
+-c [<oldbranch>] <newbranch>::
+--copy [<oldbranch>] <newbranch>::
+ Copy an existing branch <oldbranch>, which if not specified defaults
+ to the current branch, to <newbranch>. The configuration variables
+ for the <oldbranch> branch and its reflog are also copied appropriately
+ to be used with <newbranch>. Copying fails if branch <newbranch>
+ already exists, but you can use `-C` or `--copy --force` to overwrite
+ the files in existing branch <newbranch> while copying.
--C::
+-C [<oldbranch>] <newbranch>::
Shortcut for `--copy --force`.
--color[=<when>]::
@@ -311,14 +311,6 @@ superproject's "origin/main", but tracks the submodule's "origin/main".
given as a branch name, a commit-id, or a tag. If this
option is omitted, the current HEAD will be used instead.
-<oldbranch>::
- The name of an existing branch. If this option is omitted,
- the name of the current branch will be used instead.
-
-<newbranch>::
- The new name for an existing branch. The same restrictions as for
- <branchname> apply.
-
--sort=<key>::
Sort based on the key given. Prefix `-` to sort in descending
order of the value. You may use the --sort=<key> option
^ permalink raw reply related
* Re: [PATCH v2] mergetools: vimdiff: use correct tool's name when reading mergetool config
From: Junio C Hamano @ 2024-02-15 18:42 UTC (permalink / raw)
To: Kipras Melnikovas; +Cc: git, greenfoo
In-Reply-To: <20240215142002.36870-1-kipras@kipras.org>
Kipras Melnikovas <kipras@kipras.org> writes:
> Though, for backwards-compatibility, I've kept the mergetool.vimdiff
> fallback, so that people who unknowingly relied on it, won't have their
> setup broken now.
It is a good consideration, and should be documented ...
> diff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt
> index 294f61efd1..8e3d321a57 100644
> --- a/Documentation/config/mergetool.txt
> +++ b/Documentation/config/mergetool.txt
> @@ -45,10 +45,11 @@ mergetool.meld.useAutoMerge::
> value of `false` avoids using `--auto-merge` altogether, and is the
> default value.
>
> -mergetool.vimdiff.layout::
> - The vimdiff backend uses this variable to control how its split
> - windows appear. Applies even if you are using Neovim (`nvim`) or
> - gVim (`gvim`) as the merge tool. See BACKEND SPECIFIC HINTS section
> +mergetool.{g,n,}vimdiff.layout::
> + The vimdiff backend uses this variable to control how its split windows
> + appear. Use `mergetool.vimdiff` for regular Vim, `mergetool.nvimdiff` for
> + Neovim and `mergetool.gvimdiff` for gVim to configure the merge tool. See
> + BACKEND SPECIFIC HINTS section
... perhaps before "See BACKEND SPECIFIC HINTS section." E.g.
When a variant of vimdiff (vim, Neovim, or gVim) is used as
a mergetool backend, they use this variable to control how
the split windows appear.
The variable `mergetool.<variant>.layout` (where <variant>
is one of `vimdiff`, `nvimdiff`, or `gvimdiff`, depending on
what you are using) is consulted first, and if it is missing,
`mergetool.vimdiff.layout` is used as a fallback. See
BACKEND SPECIFIC HINTS section.
or something?
> diff --git a/mergetools/vimdiff b/mergetools/vimdiff
> index 06937acbf5..0e3058868a 100644
> --- a/mergetools/vimdiff
> +++ b/mergetools/vimdiff
> @@ -371,9 +371,17 @@ diff_cmd_help () {
>
>
> merge_cmd () {
> - layout=$(git config mergetool.vimdiff.layout)
> + TOOL=$1
>
> - case "$1" in
> + layout=$(git config mergetool.$TOOL.layout)
The callers of merge_cmd are careful to do
merge_cmd "$1"
so it would be a good hygiene to also quote $TOOL here, i.e.
layout=$(git config "mergetool.$TOOL.layout")
It might not matter if the caller of run_merge_cmd (which calls
merge_cmd) eventually chooses from a known set of strings hardcoded
in mergetools--lib.sh, but it is much easier to show that you are
doing the right thing without relying on such a detail of what
happens far in the code to quote what you get from the caller
appropriately.
> +
> + # backwards-compatibility:
> + if test -z "$layout"
> + then
> + layout=$(git config mergetool.vimdiff.layout)
> + fi
> +
> + case "$TOOL" in
This one is quoted properly (and TOOL=$1 at the beginning does not
require quoting). The "git config" call above is the only one that
needs to be fixed.
Thanks.
> *vimdiff)
> if test -z "$layout"
> then
>
> base-commit: 4fc51f00ef18d2c0174ab2fd39d0ee473fd144bd
^ permalink raw reply
* Re: [PATCH] use C99 declaration of variable in for() loop
From: Junio C Hamano @ 2024-02-15 18:49 UTC (permalink / raw)
To: Christian Couder; +Cc: Elia Pinto, git
In-Reply-To: <CAP8UFD2hHYeUxtXm1tNDe3tpBAif7amLbkTQpurp3w1n7uO+HQ@mail.gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
> Perhaps such changes could be accepted when they are made in only one
> file as part of a microproject though?
Yes, a microproject is not about helping this project, but is about
us investing our cycles in helping aspiring developers with small
practice material. Even though we may not want to see massive code
churn, we can view the cost of reviewing and accepting small changes
as a part of the cost to onboard new folks.
^ permalink raw reply
* Re: [PATCH] use C99 declaration of variable in for() loop
From: Elia Pinto @ 2024-02-15 18:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqcysxskd9.fsf@gitster.g>
Il giorno gio 15 feb 2024 alle ore 18:33 Junio C Hamano
<gitster@pobox.com> ha scritto:
>
> Elia Pinto <gitter.spiros@gmail.com> writes:
>
> > With the exception of cbtree.c, which would need initial
> > reworking to remove the usage of goto, it expands the
> > use of variable scope reduction in for loops as
> > permitted by the C99 standard, which was first introduced
> > in the git codebase with commit 44ba10d6.
>
> Thanks, but ...
>
> Our test balloon may have proven that nobody will be inconvenienced,
> and it does mean we can be liberal using it when we add new code or
> update existing loops "while at it", but I personally do not think
> such a code churn is very welcome.
Thank you for your reply. I can understand that, clearly. However,
this means that extensive code
refactoring contributions are never welcome. I am not saying this is a
problem, but just an observation.
Best
^ permalink raw reply
* Re: [PATCH] branch: rework the descriptions of rename and copy operations
From: Kristoffer Haugsbakk @ 2024-02-15 19:28 UTC (permalink / raw)
To: Dragan Simic; +Cc: git, Junio C Hamano, Kyle Lippincott
In-Reply-To: <3cbc78bb5729f304b30bf37a18d1762af553aa00.1708022441.git.dsimic@manjaro.org>
Hey Dragan
(I’m adding some Cc from the previous round)
On Thu, Feb 15, 2024, at 19:42, Dragan Simic wrote:
> Move the descriptions of the <oldbranch> and <newbranch> arguments to the
> descriptions of the branch rename and copy operations, where they naturally
> belong. Also, improve the descriptions of these two branch operations and,
> for completeness, describe the outcomes of forced operations.
>
> Describing the arguments together with their respective operations, instead
> of describing them separately in a rather unfortunate attempt to squeeze more
> meaning out of fewer words, flows much better and makes the git-branch(1)
> man page significantly more usable.
Excellent.
>
> The subsequent improvements shall continue this approach by either dissolving
> as many sentences from the "Description" section into the "Options" section,
> or by having those sentences converted into some kind of more readable and
> better flowing prose, as already discussed and outlined. [1][2]
>
> [1] https://lore.kernel.org/git/xmqqttmmlahf.fsf@gitster.g/T/#u
> [2] https://lore.kernel.org/git/xmqq8r4zln08.fsf@gitster.g/T/#u
Since this is a one-patch series, it wasn’t clear to me what “subsequent
improvements” was referring to without following the links. By following
the links it looks like plans have been made to improve this man page
further. Maybe the commit message could either state this intent more
assertively or commit to it less (like “we might in the future…”)? So
that the links become supplementary information instead of seemingly
filling in some blanks.
(If I read this part correctly.)
> --m::
> ---move::
> - Move/rename a branch, together with its config and reflog.
> +-m [<oldbranch>] <newbranch>::
> +--move [<oldbranch>] <newbranch>::
> + Rename an existing branch <oldbranch>, which if not specified defaults
> + to the current branch, to <newbranch>. The configuration variables
I had to read the first sentence a few times in order to understand what
the “which” part was saying (which seems to come from [1] by Junio). How
about letting it trail the sentence?
« Rename an existing branch `<oldbranch>` to `<newbranch>`, with
`<oldbranch>` defaulting to the current branch if not specified.
🔗 1: https://lore.kernel.org/git/xmqqttmmlahf.fsf@gitster.g/
> + for the <oldbranch> branch and its reflog are also renamed appropriately
> + to be used with <newbranch>. Renaming fails if branch <newbranch>
> + already exists, but you can use `-M` or `--move --force` to overwrite
> + the files in existing branch <newbranch> while renaming.
“the files” refers to the branch configuration and the reflog? People
who read the man pages might not know that the reflogs are implemented
as files and get tripped up. Maybe “to overwrite” could be left
unstated?
Or maybe I just misunderstood this part.
This patch also drops this part. Shouldn’t this be noted?
« , and a reflog entry is created to remember the branch renaming.
Right now it looks like (reads like) the reflog is moved and an entry is
not made about it.
>
> --M::
> +-M [<oldbranch>] <newbranch>::
> Shortcut for `--move --force`.
>
> --c::
> ---copy::
> - Copy a branch, together with its config and reflog.
> +-c [<oldbranch>] <newbranch>::
> +--copy [<oldbranch>] <newbranch>::
> + Copy an existing branch <oldbranch>, which if not specified defaults
> + to the current branch, to <newbranch>. The configuration variables
> + for the <oldbranch> branch and its reflog are also copied appropriately
> + to be used with <newbranch>. Copying fails if branch <newbranch>
> + already exists, but you can use `-C` or `--copy --force` to overwrite
> + the files in existing branch <newbranch> while copying.
^ permalink raw reply
* Re: [PATCH] branch: rework the descriptions of rename and copy operations
From: Dragan Simic @ 2024-02-15 19:47 UTC (permalink / raw)
To: Kristoffer Haugsbakk; +Cc: git, Junio C Hamano, Kyle Lippincott
In-Reply-To: <f9ec6d31-7158-4381-9701-06fcb33f2e83@app.fastmail.com>
Hello Kristoffer,
On 2024-02-15 20:28, Kristoffer Haugsbakk wrote:
> (I’m adding some Cc from the previous round)
Thanks, I realised that to be missing like two seconds after sending
the patch to the list. :/
> On Thu, Feb 15, 2024, at 19:42, Dragan Simic wrote:
>> Move the descriptions of the <oldbranch> and <newbranch> arguments to
>> the
>> descriptions of the branch rename and copy operations, where they
>> naturally
>> belong. Also, improve the descriptions of these two branch operations
>> and,
>> for completeness, describe the outcomes of forced operations.
>>
>> Describing the arguments together with their respective operations,
>> instead
>> of describing them separately in a rather unfortunate attempt to
>> squeeze more
>> meaning out of fewer words, flows much better and makes the
>> git-branch(1)
>> man page significantly more usable.
>
> Excellent.
Thanks, I'm glad that you like it. :)
>> The subsequent improvements shall continue this approach by either
>> dissolving
>> as many sentences from the "Description" section into the "Options"
>> section,
>> or by having those sentences converted into some kind of more readable
>> and
>> better flowing prose, as already discussed and outlined. [1][2]
>>
>> [1] https://lore.kernel.org/git/xmqqttmmlahf.fsf@gitster.g/T/#u
>> [2] https://lore.kernel.org/git/xmqq8r4zln08.fsf@gitster.g/T/#u
>
> Since this is a one-patch series, it wasn’t clear to me what
> “subsequent
> improvements” was referring to without following the links. By
> following
> the links it looks like plans have been made to improve this man page
> further. Maybe the commit message could either state this intent more
> assertively or commit to it less (like “we might in the future…”)? So
> that the links become supplementary information instead of seemingly
> filling in some blanks.
>
> (If I read this part correctly.)
It refers to the already planned further rework of the git-branch(1) man
page, which I intend to follow, but also leaves a TODO note to anyone
else
looking at the repository history, in case I end up not following the
plan
for some reason. I hope this makes it more clear.
>> --m::
>> ---move::
>> - Move/rename a branch, together with its config and reflog.
>> +-m [<oldbranch>] <newbranch>::
>> +--move [<oldbranch>] <newbranch>::
>> + Rename an existing branch <oldbranch>, which if not specified
>> defaults
>> + to the current branch, to <newbranch>. The configuration variables
>
> I had to read the first sentence a few times in order to understand
> what
> the “which” part was saying (which seems to come from [1] by Junio).
> How
> about letting it trail the sentence?
>
> « Rename an existing branch `<oldbranch>` to `<newbranch>`, with
> `<oldbranch>` defaulting to the current branch if not specified.
Makes sense, maybe even something like this for the v2:
Rename an existing branch <oldbranch> to <newbranch>; if not
specified,
<oldbranch> defaults to the current branch.
> 🔗 1: https://lore.kernel.org/git/xmqqttmmlahf.fsf@gitster.g/
>
>> + for the <oldbranch> branch and its reflog are also renamed
>> appropriately
>> + to be used with <newbranch>. Renaming fails if branch <newbranch>
>> + already exists, but you can use `-M` or `--move --force` to
>> overwrite
>> + the files in existing branch <newbranch> while renaming.
>
> “the files” refers to the branch configuration and the reflog? People
> who read the man pages might not know that the reflogs are implemented
> as files and get tripped up. Maybe “to overwrite” could be left
> unstated?
>
> Or maybe I just misunderstood this part.
Basically, the internal logic just overwrites the files, whatever the
files actually are, and issues warning messages about that. Frankly,
I'm not sure that having "--force" available is the safest possible
thing,
but it's already there and needs documenting.
Thus, "the files" refers to just that, the files in the destination
branch that pretty much gets garbled after forced operations.
Perhaps it could be worded like this in the v2:
... but you can use `-M` or `--move --force` to overwrite the
contents
of the existing branch <newbranch> while renaming.
> This patch also drops this part. Shouldn’t this be noted?
>
> « , and a reflog entry is created to remember the branch renaming.
>
> Right now it looks like (reads like) the reflog is moved and an entry
> is
> not made about it.
Good point, thanks. I'll address that in the v2.
>> --M::
>> +-M [<oldbranch>] <newbranch>::
>> Shortcut for `--move --force`.
>>
>> --c::
>> ---copy::
>> - Copy a branch, together with its config and reflog.
>> +-c [<oldbranch>] <newbranch>::
>> +--copy [<oldbranch>] <newbranch>::
>> + Copy an existing branch <oldbranch>, which if not specified defaults
>> + to the current branch, to <newbranch>. The configuration variables
>> + for the <oldbranch> branch and its reflog are also copied
>> appropriately
>> + to be used with <newbranch>. Copying fails if branch <newbranch>
>> + already exists, but you can use `-C` or `--copy --force` to
>> overwrite
>> + the files in existing branch <newbranch> while copying.
^ permalink raw reply
* Re: [PATCH] use C99 declaration of variable in for() loop
From: Junio C Hamano @ 2024-02-15 20:07 UTC (permalink / raw)
To: Elia Pinto; +Cc: git
In-Reply-To: <CA+EOSBktLGnzi+DjOTVv9_cVNsM_fjEKiF9kWnaYrGtvGJ-Kdg@mail.gmail.com>
Elia Pinto <gitter.spiros@gmail.com> writes:
> Thank you for your reply. I can understand that, clearly. However,
> this means that extensive code
> refactoring contributions are never welcome. I am not saying this is a
> problem, but just an observation.
Such changes can happen and have happened when the benefit of such
code churn outweighs the cost of reviewing *and* cost of updating or
adjusting in-flight topics that may already or may not yet be in my
tree. Coccinelle-driven patches that can be mechanically reproduced
and whose validity can be trusted can be one way to reduce the review
and maintenance cost for such a tree-wide change.
^ permalink raw reply
* Re: [PATCH] branch: rework the descriptions of rename and copy operations
From: Junio C Hamano @ 2024-02-15 20:41 UTC (permalink / raw)
To: Kristoffer Haugsbakk; +Cc: Dragan Simic, git, Kyle Lippincott
In-Reply-To: <f9ec6d31-7158-4381-9701-06fcb33f2e83@app.fastmail.com>
"Kristoffer Haugsbakk" <code@khaugsbakk.name> writes:
> (If I read this part correctly.)
>
>> --m::
>> ---move::
>> - Move/rename a branch, together with its config and reflog.
>> +-m [<oldbranch>] <newbranch>::
>> +--move [<oldbranch>] <newbranch>::
>> + Rename an existing branch <oldbranch>, which if not specified defaults
>> + to the current branch, to <newbranch>. The configuration variables
>
> I had to read the first sentence a few times in order to understand what
> the “which” part was saying (which seems to come from [1] by Junio). How
> about letting it trail the sentence?
>
> « Rename an existing branch `<oldbranch>` to `<newbranch>`, with
> `<oldbranch>` defaulting to the current branch if not specified.
Yeah, insertion of "if not specified" in the middle made the
resulting text unnecessarily hard to follow, and moving it to the
end is a fine fix. But I think we can just drop it (in other words,
we already said "defaulting to" and that should be sufficient).
>> + for the <oldbranch> branch and its reflog are also renamed appropriately
>> + to be used with <newbranch>. Renaming fails if branch <newbranch>
>> + already exists, but you can use `-M` or `--move --force` to overwrite
>> + the files in existing branch <newbranch> while renaming.
>
> “the files” refers to the branch configuration and the reflog? People
> who read the man pages might not know that the reflogs are implemented
> as files and get tripped up. Maybe “to overwrite” could be left
> unstated?
Yeah, "overwrite the files in existing branch" can be mislead in
other ways, including "in a similar way that 'checkout -f <other>'
can overwrite the files with local modifications in the working tree
with the <other> branch we are switching to". We'd be better off
not mentioning files at all [*].
... but you can use ... to clobber an existing `<newbranch>`.
would be sufficient. If the verb "clobber" is unfamiliar to some
readers, "overwrite" is also fine. The most important part from the
end-user's point of view is that whatever data associated with the
name <newbranch> is now gone, and replaced by what was associated
with the name <oldbranch>. How we stored such data associated with
each branch is immaterial.
Side note: Especially because we are not married to the
files backend that stores one file per branch under
.git/logs/refs/heads/ and .git/refs/heads/ directories
forever. With reftable backend, there is no such files
specific to <newbranch> to be overwritten.
Another thing. The existence of a <newbranch> is not the only case
we fail "git branch -m [<oldbranch>] <newbranch>", but the mention
of this particular safety valve is to tell readers that forcing with
`-M` is how to override the safety. We want to be absolutely clear
that we are not trying to exhaustively enumerate failure modes [*]
to our readers, and I am not sure if we succeeded in the proposed
text.
Side note: In other words, there are other ways the command
can fail, and `-M` may not be a way to "fix" these failures.
> This patch also drops this part. Shouldn’t this be noted?
>
> « , and a reflog entry is created to remember the branch renaming.
>
> Right now it looks like (reads like) the reflog is moved and an entry is
> not made about it.
True. This was missing in my "or something like that" illustration
and the patch copies without checking the original. It should be
resurrected.
Thanks, both for writing and carefully reading. Removal of
<newbranch> and <oldbranch> from the OPTIONS enumaration is really
good, too.
^ permalink raw reply
* Re: [PATCH v2] mergetools: vimdiff: use correct tool's name when reading mergetool config
From: Fernando Ramos @ 2024-02-15 20:43 UTC (permalink / raw)
To: Kipras Melnikovas; +Cc: git
In-Reply-To: <20240215142002.36870-1-kipras@kipras.org>
On 24/02/15 04:20PM, Kipras Melnikovas wrote:
> The /mergetools/vimdiff script, which handles both vimdiff, nvimdiff
> and gvimdiff mergetools (the latter 2 simply source the vimdiff script), has a
> function merge_cmd() which read the layout variable from git config, and it
> would always read the value of mergetool.**vimdiff**.layout, instead of the
> mergetool being currently used (vimdiff or nvimdiff or gvimdiff).
You are 100% right. I completely overlooked this detail in the original
implementation.
Thanks for the fix!
^ permalink raw reply
* Re: [PATCH] git: --no-lazy-fetch option
From: Linus Arver @ 2024-02-15 20:59 UTC (permalink / raw)
To: Jeff King, Junio C Hamano; +Cc: git, Christian Couder
In-Reply-To: <20240215053056.GD2821179@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Thu, Feb 08, 2024 at 03:17:31PM -0800, Junio C Hamano wrote:
>
>> Sometimes, especially during tests of low level machinery, it is
>> handy to have a way to disable lazy fetching of objects. This
>> allows us to say, for example, "git cat-file -e <object-name>", to
>> see if the object is locally available.
>
> That seems like a good feature, but...
>
>> @@ -186,6 +187,8 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
>> use_pager = 0;
>> if (envchanged)
>> *envchanged = 1;
>> + } else if (!strcmp(cmd, "--no-lazy-fetch")) {
>> + fetch_if_missing = 0;
>> } else if (!strcmp(cmd, "--no-replace-objects")) {
>> disable_replace_refs();
>> setenv(NO_REPLACE_OBJECTS_ENVIRONMENT, "1", 1);
>
> This will only help builtin commands, and even then only the top-level
> one. If I run "git --no-lazy-fetch foo" and "foo" is a script or an
> alias, I'd expect it to still take effect. Ditto for sub-commands kicked
> off by a builtin (say, a "rev-list" connectivity check caused by a
> fetch).
>
> So this probably needs to be modeled after --no-replace-objects, etc,
> where we set an environment variable that makes it to child processes.
Thanks for the helpful explanation, very much appreciated.
^ permalink raw reply
* Re: [PATCH] branch: rework the descriptions of rename and copy operations
From: Dragan Simic @ 2024-02-15 21:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kristoffer Haugsbakk, git, Kyle Lippincott
In-Reply-To: <xmqqedddo3ym.fsf@gitster.g>
Hello Junio,
On 2024-02-15 21:41, Junio C Hamano wrote:
> "Kristoffer Haugsbakk" <code@khaugsbakk.name> writes:
>> (If I read this part correctly.)
>>
>>> --m::
>>> ---move::
>>> - Move/rename a branch, together with its config and reflog.
>>> +-m [<oldbranch>] <newbranch>::
>>> +--move [<oldbranch>] <newbranch>::
>>> + Rename an existing branch <oldbranch>, which if not specified
>>> defaults
>>> + to the current branch, to <newbranch>. The configuration variables
>>
>> I had to read the first sentence a few times in order to understand
>> what
>> the “which” part was saying (which seems to come from [1] by Junio).
>> How
>> about letting it trail the sentence?
>>
>> « Rename an existing branch `<oldbranch>` to `<newbranch>`, with
>> `<oldbranch>` defaulting to the current branch if not specified.
>
> Yeah, insertion of "if not specified" in the middle made the
> resulting text unnecessarily hard to follow, and moving it to the
> end is a fine fix. But I think we can just drop it (in other words,
> we already said "defaulting to" and that should be sufficient).
This is what seems most readable to me:
Rename an existing branch `<oldbranch>` to `<newbranch>`; if left
unspecified, `<oldbranch>` defaults to the current branch.
>>> + for the <oldbranch> branch and its reflog are also renamed
>>> appropriately
>>> + to be used with <newbranch>. Renaming fails if branch <newbranch>
>>> + already exists, but you can use `-M` or `--move --force` to
>>> overwrite
>>> + the files in existing branch <newbranch> while renaming.
>>
>> “the files” refers to the branch configuration and the reflog? People
>> who read the man pages might not know that the reflogs are implemented
>> as files and get tripped up. Maybe “to overwrite” could be left
>> unstated?
>
> Yeah, "overwrite the files in existing branch" can be mislead in
> other ways, including "in a similar way that 'checkout -f <other>'
> can overwrite the files with local modifications in the working tree
> with the <other> branch we are switching to". We'd be better off
> not mentioning files at all [*].
>
> ... but you can use ... to clobber an existing `<newbranch>`.
>
> would be sufficient. If the verb "clobber" is unfamiliar to some
> readers, "overwrite" is also fine. The most important part from the
> end-user's point of view is that whatever data associated with the
> name <newbranch> is now gone, and replaced by what was associated
> with the name <oldbranch>. How we stored such data associated with
> each branch is immaterial.
>
> Side note: Especially because we are not married to the
> files backend that stores one file per branch under
> .git/logs/refs/heads/ and .git/refs/heads/ directories
> forever. With reftable backend, there is no such files
> specific to <newbranch> to be overwritten.
To me, using "clobber" actually doesn't make it more clear. How
about wording it this way:
Renaming fails if branch `<newbranch>` already exists, but `-M`
or `--move --force` can be used to overwrite the contents of the
existing branch `<newbranch>` while renaming.
To me, "overwriting an existing branch" means something like
"deleting an existing branch first, before doing anything else".
Just like in case of overwriting an existing file, which gets
deleted first, which most users are familiar with.
On the other hand, "overwriting the contents of an existing
branch", at least to me, means that the branch isn't deleted first,
but the new branch is "layered" onto it instead, overwriting some
or all of its contents.
Additionally, "its contents" removes the direct link between the
files and the branches, which should make the description future
proof.
> Another thing. The existence of a <newbranch> is not the only case
> we fail "git branch -m [<oldbranch>] <newbranch>", but the mention
> of this particular safety valve is to tell readers that forcing with
> `-M` is how to override the safety. We want to be absolutely clear
> that we are not trying to exhaustively enumerate failure modes [*]
> to our readers, and I am not sure if we succeeded in the proposed
> text.
>
> Side note: In other words, there are other ways the command
> can fail, and `-M` may not be a way to "fix" these failures.
>
>> This patch also drops this part. Shouldn’t this be noted?
>>
>> « , and a reflog entry is created to remember the branch renaming.
>>
>> Right now it looks like (reads like) the reflog is moved and an entry
>> is
>> not made about it.
>
> True. This was missing in my "or something like that" illustration
> and the patch copies without checking the original. It should be
> resurrected.
Already brought this back for the v2. It was omitted in the v1 by
an honest mistake.
> Thanks, both for writing and carefully reading. Removal of
> <newbranch> and <oldbranch> from the OPTIONS enumaration is really
> good, too.
I'm glad that you're happy with the improvements so far. :)
^ permalink raw reply
* RE: Hello question on Git for Windows 2.43.0 - GUID and/or SWID tag for this title
From: Christian Castro @ 2024-02-15 21:40 UTC (permalink / raw)
To: Matthias Aßhauer; +Cc: git@vger.kernel.org
In-Reply-To: <DB9P250MB06926B4A6997EF6F866296ADA54A2@DB9P250MB0692.EURP250.PROD.OUTLOOK.COM>
Hello Matthias,
Thank you for your reply.
Question: Are you a Git for Windows developer, open-source contributor or ? I ask because I will contact the manufacturer of our inventory product and provide them your feedback. But I'd like to know what your role is with Git for Windows for as of now I just have a reply from someone named Matthias from a live.de email domain. I hope you understand. Truly no offense meant on my part.
Therefore, please let me know what your role is with Git for Windows so I can send this feedback accordingly and continue working on with our software inventory vendor on the issue.
Best regards,
Christian
-----Original Message-----
From: Matthias Aßhauer <mha1993@live.de>
Sent: Saturday, February 10, 2024 2:40 AM
To: Christian Castro <christian.castro@dlhcorp.com>
Cc: git@vger.kernel.org
Subject: Re: Hello question on Git for Windows 2.43.0 - GUID and/or SWID tag for this title
[You don't often get email from mha1993@live.de. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
On Fri, 2 Feb 2024, Christian Castro wrote:
> Hello Git for Windows,
>
>
> I have a question on the GUID and/or SWID tag for Git for Windows 2.43.0.
>
> Can you tell me where in the product the GUID and/or SWID tag would be stored in Windows for Git for Windows 2.43.0?
> Our scanning software has detected both 2.39.2 and 2.43.0 on the same Windows but this is not so, only 2.43.0 is installed.
> This was an upgrade from 2.39.2 though so not sure if that is messed up the results somehow.
>
> I've looked in C:\ProgramData and there are no regid folders not regid.xyz files exist for this product (for the SWID tag).
>
> The Windows registry also does not have a GUID information for this product located under:
> HKLM\Software\GitForWindows
> HKLM\Software\Microsoft\Windows\CurrentVersion\Uninstall\Git_is1
>
>
> If you've included a GUID and/or SWID tag for your product somewhere in the installation process, can you please tell me where these are stored under Windows so I can fix? I'd appreciate it.
>
Neither of those things really exist in Git for Windows. Well there are GUIDs, but no GUID that is used in the way you'd expect from an MSI installer.
The Git for Windows installer is an innosetup based EXE installer.
Innosetup doesn't have these concepts. There have been efforts to introduce an MSI installer over the years, but they've all fizzled out.
As for SWID tags, you're the first person to mention them in almost 15 years of ISO/IEC 19770-2 existing.
>
> Thank you,
>
> Christian Castro
> Sr. Systems Administrator
> Office: 301-628-3551
> DLHcorp.com
>
>
>
> ****WARNING**** This email message (including any attachments) are to be treated as confidential/proprietary and may contain copyrighted or other legally protected information. It is intended only for the addressee(s) identified above. If you are not the addressee(s), or an employee or agent of the addressee(s), please note that any dissemination, distribution, or copying of this communication is strictly prohibited. If you have received this information in error, please destroy the information and notify the sender of the error. Thank you.
>
****WARNING**** This email message (including any attachments) are to be treated as confidential/proprietary and may contain copyrighted or other legally protected information. It is intended only for the addressee(s) identified above. If you are not the addressee(s), or an employee or agent of the addressee(s), please note that any dissemination, distribution, or copying of this communication is strictly prohibited. If you have received this information in error, please destroy the information and notify the sender of the error. Thank you.
^ permalink raw reply
* Re: [PATCH] branch: rework the descriptions of rename and copy operations
From: Rubén Justo @ 2024-02-15 21:52 UTC (permalink / raw)
To: Dragan Simic, git
In-Reply-To: <3cbc78bb5729f304b30bf37a18d1762af553aa00.1708022441.git.dsimic@manjaro.org>
On 15-feb-2024 19:42:32, Dragan Simic wrote:
> Move the descriptions of the <oldbranch> and <newbranch> arguments to the
> descriptions of the branch rename and copy operations, where they naturally
> belong.
Thank you Dragan for working on this.
Let me chime in just to say that maybe another terms could be considered
here; like: "<branchname>" and "<newbranchname>" (maybe too long...) or
so.
I have no problem with the current terms, but "<branchname>" can be a
sensible choice here as it is already being used for other commands
where, and this may help overall, the consideration: "if ommited, the
current branch is considered" also applies.
> Also, improve the descriptions of these two branch operations and,
> for completeness, describe the outcomes of forced operations.
>
> Describing the arguments together with their respective operations, instead
> of describing them separately in a rather unfortunate attempt to squeeze more
> meaning out of fewer words, flows much better and makes the git-branch(1)
> man page significantly more usable.
>
> The subsequent improvements shall continue this approach by either dissolving
> as many sentences from the "Description" section into the "Options" section,
> or by having those sentences converted into some kind of more readable and
> better flowing prose, as already discussed and outlined. [1][2]
>
> [1] https://lore.kernel.org/git/xmqqttmmlahf.fsf@gitster.g/T/#u
> [2] https://lore.kernel.org/git/xmqq8r4zln08.fsf@gitster.g/T/#u
>
> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
> ---
>
> Notes:
> This patch was originally named "branch: clarify <oldbranch> and <newbranch>
> terms further", submitted and discussed in another thread, [3] but the nature
> of the patch has changed, causing the patch subject to be adjusted to match.
>
> Consequently, this is effectively version 2 of the patch, which includes
> detailed feedback from Kyle and Junio, who suggested moving/adding the
> argument descriptions to their respective commands. This resulted in more
> significant changes to the contents of the git-branch(1) man page, in an
> attempt to make it more readable.
>
> [3] https://lore.kernel.org/git/e2eb777bca8ffeec42bdd684837d28dd52cfc9c3.1707136999.git.dsimic@manjaro.org/T/#u
>
> Documentation/git-branch.txt | 44 +++++++++++++++---------------------
> 1 file changed, 18 insertions(+), 26 deletions(-)
>
> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
> index 0b0844293235..370ea43c0380 100644
> --- a/Documentation/git-branch.txt
> +++ b/Documentation/git-branch.txt
> @@ -72,16 +72,6 @@ the remote-tracking branch. This behavior may be changed via the global
> overridden by using the `--track` and `--no-track` options, and
> changed later using `git branch --set-upstream-to`.
>
> -With a `-m` or `-M` option, <oldbranch> will be renamed to <newbranch>.
> -If <oldbranch> had a corresponding reflog, it is renamed to match
> -<newbranch>, and a reflog entry is created to remember the branch
> -renaming. If <newbranch> exists, -M must be used to force the rename
> -to happen.
> -
> -The `-c` and `-C` options have the exact same semantics as `-m` and
> -`-M`, except instead of the branch being renamed, it will be copied to a
> -new name, along with its config and reflog.
> -
> With a `-d` or `-D` option, `<branchname>` will be deleted. You may
> specify more than one branch for deletion. If the branch currently
> has a reflog then the reflog will also be deleted.
> @@ -128,18 +118,28 @@ Note that 'git branch -f <branchname> [<start-point>]', even with '-f',
> refuses to change an existing branch `<branchname>` that is checked out
> in another worktree linked to the same repository.
>
> --m::
> ---move::
> - Move/rename a branch, together with its config and reflog.
> +-m [<oldbranch>] <newbranch>::
> +--move [<oldbranch>] <newbranch>::
> + Rename an existing branch <oldbranch>, which if not specified defaults
> + to the current branch, to <newbranch>. The configuration variables
> + for the <oldbranch> branch and its reflog are also renamed appropriately
> + to be used with <newbranch>. Renaming fails if branch <newbranch>
> + already exists, but you can use `-M` or `--move --force` to overwrite
> + the files in existing branch <newbranch> while renaming.
>
> --M::
> +-M [<oldbranch>] <newbranch>::
> Shortcut for `--move --force`.
>
> --c::
> ---copy::
> - Copy a branch, together with its config and reflog.
> +-c [<oldbranch>] <newbranch>::
> +--copy [<oldbranch>] <newbranch>::
> + Copy an existing branch <oldbranch>, which if not specified defaults
> + to the current branch, to <newbranch>. The configuration variables
> + for the <oldbranch> branch and its reflog are also copied appropriately
> + to be used with <newbranch>. Copying fails if branch <newbranch>
> + already exists, but you can use `-C` or `--copy --force` to overwrite
> + the files in existing branch <newbranch> while copying.
>
> --C::
> +-C [<oldbranch>] <newbranch>::
> Shortcut for `--copy --force`.
>
> --color[=<when>]::
> @@ -311,14 +311,6 @@ superproject's "origin/main", but tracks the submodule's "origin/main".
> given as a branch name, a commit-id, or a tag. If this
> option is omitted, the current HEAD will be used instead.
>
> -<oldbranch>::
> - The name of an existing branch. If this option is omitted,
> - the name of the current branch will be used instead.
> -
> -<newbranch>::
> - The new name for an existing branch. The same restrictions as for
> - <branchname> apply.
> -
> --sort=<key>::
> Sort based on the key given. Prefix `-` to sort in descending
> order of the value. You may use the --sort=<key> option
^ permalink raw reply
* Re: [PATCH] branch: rework the descriptions of rename and copy operations
From: Junio C Hamano @ 2024-02-15 22:13 UTC (permalink / raw)
To: Rubén Justo; +Cc: Dragan Simic, git
In-Reply-To: <e8fdd057-2670-4c93-b362-202a339d5f49@gmail.com>
Rubén Justo <rjusto@gmail.com> writes:
> On 15-feb-2024 19:42:32, Dragan Simic wrote:
>
>> Move the descriptions of the <oldbranch> and <newbranch> arguments to the
>> descriptions of the branch rename and copy operations, where they naturally
>> belong.
>
> Thank you Dragan for working on this.
>
> Let me chime in just to say that maybe another terms could be considered
> here; like: "<branchname>" and "<newbranchname>" (maybe too long...) or
> so.
>
> I have no problem with the current terms, but "<branchname>" can be a
> sensible choice here as it is already being used for other commands
> where, and this may help overall, the consideration: "if ommited, the
> current branch is considered" also applies.
Actually, we should go in the opposite direction. When the use of
names are localized in a narrower context, they can be shortened
without losing clarity. For example:
-m [<old>] <new>::
rename the <old> branch (defaults to the current one) to
<new>.
is just as clear as the same description with <oldbranch> and
<newbranch>. With the original text without any of the suggested
changes, <oldbranch> and <newbranch> appeared very far from the
context they are used in (i.e. the description for -m and -c), and
it may have helped readers to tell that these are names of branches.
But if the context is clear that we are talking about "renaming"
branches, there is not as much added benefit to say "branch" in
these names as in the current text.
^ permalink raw reply
* Re: [PATCH] branch: rework the descriptions of rename and copy operations
From: Dragan Simic @ 2024-02-15 22:27 UTC (permalink / raw)
To: Rubén Justo; +Cc: git
In-Reply-To: <e8fdd057-2670-4c93-b362-202a339d5f49@gmail.com>
Hello Ruben and Junio,
On 2024-02-15 22:52, Rubén Justo wrote:
> On 15-feb-2024 19:42:32, Dragan Simic wrote:
>
>> Move the descriptions of the <oldbranch> and <newbranch> arguments to
>> the
>> descriptions of the branch rename and copy operations, where they
>> naturally
>> belong.
>
> Thank you Dragan for working on this.
Thank you, and everyone else, for the reviews and suggestions.
> Let me chime in just to say that maybe another terms could be
> considered
> here; like: "<branchname>" and "<newbranchname>" (maybe too long...)
> or
> so.
>
> I have no problem with the current terms, but "<branchname>" can be a
> sensible choice here as it is already being used for other commands
> where, and this may help overall, the consideration: "if ommited, the
> current branch is considered" also applies.
Actually, I'd agree with Junio's reply that suggested using even
shorter terms. Just like "<oldbranch>" and "<newbranch>" can safely
be shortened to "<old>" and "<new>", respectively, "<branchname>"
can also be shortened to "<name>".
It's all about the context, which is improved by moving the descriptions
of the arguments closer to the descriptions of the commands.
Though, I'd prefer that we keep "<oldbranch>" and "<newbranch>" (and
"<branchname>") for now, for the sake of consistency, and I'd get them
shortened in the future patches.
>> Also, improve the descriptions of these two branch operations and,
>> for completeness, describe the outcomes of forced operations.
>>
>> Describing the arguments together with their respective operations,
>> instead
>> of describing them separately in a rather unfortunate attempt to
>> squeeze more
>> meaning out of fewer words, flows much better and makes the
>> git-branch(1)
>> man page significantly more usable.
>>
>> The subsequent improvements shall continue this approach by either
>> dissolving
>> as many sentences from the "Description" section into the "Options"
>> section,
>> or by having those sentences converted into some kind of more readable
>> and
>> better flowing prose, as already discussed and outlined. [1][2]
>>
>> [1] https://lore.kernel.org/git/xmqqttmmlahf.fsf@gitster.g/T/#u
>> [2] https://lore.kernel.org/git/xmqq8r4zln08.fsf@gitster.g/T/#u
>>
>> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
>> ---
>>
>> Notes:
>> This patch was originally named "branch: clarify <oldbranch> and
>> <newbranch>
>> terms further", submitted and discussed in another thread, [3] but
>> the nature
>> of the patch has changed, causing the patch subject to be adjusted
>> to match.
>>
>> Consequently, this is effectively version 2 of the patch, which
>> includes
>> detailed feedback from Kyle and Junio, who suggested moving/adding
>> the
>> argument descriptions to their respective commands. This resulted
>> in more
>> significant changes to the contents of the git-branch(1) man page,
>> in an
>> attempt to make it more readable.
>>
>> [3]
>> https://lore.kernel.org/git/e2eb777bca8ffeec42bdd684837d28dd52cfc9c3.1707136999.git.dsimic@manjaro.org/T/#u
>>
>> Documentation/git-branch.txt | 44
>> +++++++++++++++---------------------
>> 1 file changed, 18 insertions(+), 26 deletions(-)
>>
>> diff --git a/Documentation/git-branch.txt
>> b/Documentation/git-branch.txt
>> index 0b0844293235..370ea43c0380 100644
>> --- a/Documentation/git-branch.txt
>> +++ b/Documentation/git-branch.txt
>> @@ -72,16 +72,6 @@ the remote-tracking branch. This behavior may be
>> changed via the global
>> overridden by using the `--track` and `--no-track` options, and
>> changed later using `git branch --set-upstream-to`.
>>
>> -With a `-m` or `-M` option, <oldbranch> will be renamed to
>> <newbranch>.
>> -If <oldbranch> had a corresponding reflog, it is renamed to match
>> -<newbranch>, and a reflog entry is created to remember the branch
>> -renaming. If <newbranch> exists, -M must be used to force the rename
>> -to happen.
>> -
>> -The `-c` and `-C` options have the exact same semantics as `-m` and
>> -`-M`, except instead of the branch being renamed, it will be copied
>> to a
>> -new name, along with its config and reflog.
>> -
>> With a `-d` or `-D` option, `<branchname>` will be deleted. You may
>> specify more than one branch for deletion. If the branch currently
>> has a reflog then the reflog will also be deleted.
>> @@ -128,18 +118,28 @@ Note that 'git branch -f <branchname>
>> [<start-point>]', even with '-f',
>> refuses to change an existing branch `<branchname>` that is checked
>> out
>> in another worktree linked to the same repository.
>>
>> --m::
>> ---move::
>> - Move/rename a branch, together with its config and reflog.
>> +-m [<oldbranch>] <newbranch>::
>> +--move [<oldbranch>] <newbranch>::
>> + Rename an existing branch <oldbranch>, which if not specified
>> defaults
>> + to the current branch, to <newbranch>. The configuration variables
>> + for the <oldbranch> branch and its reflog are also renamed
>> appropriately
>> + to be used with <newbranch>. Renaming fails if branch <newbranch>
>> + already exists, but you can use `-M` or `--move --force` to
>> overwrite
>> + the files in existing branch <newbranch> while renaming.
>>
>> --M::
>> +-M [<oldbranch>] <newbranch>::
>> Shortcut for `--move --force`.
>>
>> --c::
>> ---copy::
>> - Copy a branch, together with its config and reflog.
>> +-c [<oldbranch>] <newbranch>::
>> +--copy [<oldbranch>] <newbranch>::
>> + Copy an existing branch <oldbranch>, which if not specified defaults
>> + to the current branch, to <newbranch>. The configuration variables
>> + for the <oldbranch> branch and its reflog are also copied
>> appropriately
>> + to be used with <newbranch>. Copying fails if branch <newbranch>
>> + already exists, but you can use `-C` or `--copy --force` to
>> overwrite
>> + the files in existing branch <newbranch> while copying.
>>
>> --C::
>> +-C [<oldbranch>] <newbranch>::
>> Shortcut for `--copy --force`.
>>
>> --color[=<when>]::
>> @@ -311,14 +311,6 @@ superproject's "origin/main", but tracks the
>> submodule's "origin/main".
>> given as a branch name, a commit-id, or a tag. If this
>> option is omitted, the current HEAD will be used instead.
>>
>> -<oldbranch>::
>> - The name of an existing branch. If this option is omitted,
>> - the name of the current branch will be used instead.
>> -
>> -<newbranch>::
>> - The new name for an existing branch. The same restrictions as for
>> - <branchname> apply.
>> -
>> --sort=<key>::
>> Sort based on the key given. Prefix `-` to sort in descending
>> order of the value. You may use the --sort=<key> option
^ permalink raw reply
* Re: [PATCH] branch: rework the descriptions of rename and copy operations
From: Kyle Lippincott @ 2024-02-15 22:31 UTC (permalink / raw)
To: Dragan Simic; +Cc: git
In-Reply-To: <3cbc78bb5729f304b30bf37a18d1762af553aa00.1708022441.git.dsimic@manjaro.org>
On Thu, Feb 15, 2024 at 10:43 AM Dragan Simic <dsimic@manjaro.org> wrote:
>
> Move the descriptions of the <oldbranch> and <newbranch> arguments to the
> descriptions of the branch rename and copy operations, where they naturally
> belong. Also, improve the descriptions of these two branch operations and,
> for completeness, describe the outcomes of forced operations.
>
> Describing the arguments together with their respective operations, instead
> of describing them separately in a rather unfortunate attempt to squeeze more
> meaning out of fewer words, flows much better and makes the git-branch(1)
> man page significantly more usable.
>
> The subsequent improvements shall continue this approach by either dissolving
> as many sentences from the "Description" section into the "Options" section,
> or by having those sentences converted into some kind of more readable and
> better flowing prose, as already discussed and outlined. [1][2]
>
> [1] https://lore.kernel.org/git/xmqqttmmlahf.fsf@gitster.g/T/#u
> [2] https://lore.kernel.org/git/xmqq8r4zln08.fsf@gitster.g/T/#u
>
> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
> ---
>
> Notes:
> This patch was originally named "branch: clarify <oldbranch> and <newbranch>
> terms further", submitted and discussed in another thread, [3] but the nature
> of the patch has changed, causing the patch subject to be adjusted to match.
>
> Consequently, this is effectively version 2 of the patch, which includes
> detailed feedback from Kyle and Junio, who suggested moving/adding the
> argument descriptions to their respective commands. This resulted in more
> significant changes to the contents of the git-branch(1) man page, in an
> attempt to make it more readable.
>
> [3] https://lore.kernel.org/git/e2eb777bca8ffeec42bdd684837d28dd52cfc9c3.1707136999.git.dsimic@manjaro.org/T/#u
>
> Documentation/git-branch.txt | 44 +++++++++++++++---------------------
> 1 file changed, 18 insertions(+), 26 deletions(-)
Thanks! I think this is a great improvement to this document.
>
> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
> index 0b0844293235..370ea43c0380 100644
> --- a/Documentation/git-branch.txt
> +++ b/Documentation/git-branch.txt
> @@ -72,16 +72,6 @@ the remote-tracking branch. This behavior may be changed via the global
> overridden by using the `--track` and `--no-track` options, and
> changed later using `git branch --set-upstream-to`.
>
> -With a `-m` or `-M` option, <oldbranch> will be renamed to <newbranch>.
> -If <oldbranch> had a corresponding reflog, it is renamed to match
> -<newbranch>, and a reflog entry is created to remember the branch
> -renaming. If <newbranch> exists, -M must be used to force the rename
> -to happen.
> -
> -The `-c` and `-C` options have the exact same semantics as `-m` and
> -`-M`, except instead of the branch being renamed, it will be copied to a
> -new name, along with its config and reflog.
> -
> With a `-d` or `-D` option, `<branchname>` will be deleted. You may
> specify more than one branch for deletion. If the branch currently
> has a reflog then the reflog will also be deleted.
> @@ -128,18 +118,28 @@ Note that 'git branch -f <branchname> [<start-point>]', even with '-f',
> refuses to change an existing branch `<branchname>` that is checked out
> in another worktree linked to the same repository.
>
> --m::
> ---move::
> - Move/rename a branch, together with its config and reflog.
> +-m [<oldbranch>] <newbranch>::
> +--move [<oldbranch>] <newbranch>::
> + Rename an existing branch <oldbranch>, which if not specified defaults
> + to the current branch, to <newbranch>. The configuration variables
Very minor nit: the prevailing style in this document appears to be to
have a single space after the period, but it's definitely
inconsistent. I don't see anything in Documentation/CodingGuidelines
suggesting one way or the other, so don't consider this comment as
blocking, just informational. If we want to achieve (eventual)
consistency, we may want to use a single space after the period now.
> + for the <oldbranch> branch and its reflog are also renamed appropriately
> + to be used with <newbranch>. Renaming fails if branch <newbranch>
> + already exists, but you can use `-M` or `--move --force` to overwrite
> + the files in existing branch <newbranch> while renaming.
>
> --M::
> +-M [<oldbranch>] <newbranch>::
> Shortcut for `--move --force`.
>
> --c::
> ---copy::
> - Copy a branch, together with its config and reflog.
> +-c [<oldbranch>] <newbranch>::
> +--copy [<oldbranch>] <newbranch>::
> + Copy an existing branch <oldbranch>, which if not specified defaults
> + to the current branch, to <newbranch>. The configuration variables
> + for the <oldbranch> branch and its reflog are also copied appropriately
> + to be used with <newbranch>. Copying fails if branch <newbranch>
> + already exists, but you can use `-C` or `--copy --force` to overwrite
> + the files in existing branch <newbranch> while copying.
>
> --C::
> +-C [<oldbranch>] <newbranch>::
> Shortcut for `--copy --force`.
>
> --color[=<when>]::
> @@ -311,14 +311,6 @@ superproject's "origin/main", but tracks the submodule's "origin/main".
> given as a branch name, a commit-id, or a tag. If this
> option is omitted, the current HEAD will be used instead.
>
> -<oldbranch>::
> - The name of an existing branch. If this option is omitted,
> - the name of the current branch will be used instead.
> -
> -<newbranch>::
> - The new name for an existing branch. The same restrictions as for
> - <branchname> apply.
> -
> --sort=<key>::
> Sort based on the key given. Prefix `-` to sort in descending
> order of the value. You may use the --sort=<key> option
>
^ permalink raw reply
* Re: [PATCH] branch: rework the descriptions of rename and copy operations
From: Dragan Simic @ 2024-02-15 22:38 UTC (permalink / raw)
To: Kyle Lippincott; +Cc: git
In-Reply-To: <CAO_smVioz0izmunDRyHjU_7GGz-_Om_6AVw2CZGFyb4ZF8yedg@mail.gmail.com>
Hello Kyle,
On 2024-02-15 23:31, Kyle Lippincott wrote:
> On Thu, Feb 15, 2024 at 10:43 AM Dragan Simic <dsimic@manjaro.org>
> wrote:
>>
>> Move the descriptions of the <oldbranch> and <newbranch> arguments to
>> the
>> descriptions of the branch rename and copy operations, where they
>> naturally
>> belong. Also, improve the descriptions of these two branch operations
>> and,
>> for completeness, describe the outcomes of forced operations.
>>
>> Describing the arguments together with their respective operations,
>> instead
>> of describing them separately in a rather unfortunate attempt to
>> squeeze more
>> meaning out of fewer words, flows much better and makes the
>> git-branch(1)
>> man page significantly more usable.
>>
>> The subsequent improvements shall continue this approach by either
>> dissolving
>> as many sentences from the "Description" section into the "Options"
>> section,
>> or by having those sentences converted into some kind of more readable
>> and
>> better flowing prose, as already discussed and outlined. [1][2]
>>
>> [1] https://lore.kernel.org/git/xmqqttmmlahf.fsf@gitster.g/T/#u
>> [2] https://lore.kernel.org/git/xmqq8r4zln08.fsf@gitster.g/T/#u
>>
>> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
>> ---
>>
>> Notes:
>> This patch was originally named "branch: clarify <oldbranch> and
>> <newbranch>
>> terms further", submitted and discussed in another thread, [3] but
>> the nature
>> of the patch has changed, causing the patch subject to be adjusted
>> to match.
>>
>> Consequently, this is effectively version 2 of the patch, which
>> includes
>> detailed feedback from Kyle and Junio, who suggested moving/adding
>> the
>> argument descriptions to their respective commands. This resulted
>> in more
>> significant changes to the contents of the git-branch(1) man page,
>> in an
>> attempt to make it more readable.
>>
>> [3]
>> https://lore.kernel.org/git/e2eb777bca8ffeec42bdd684837d28dd52cfc9c3.1707136999.git.dsimic@manjaro.org/T/#u
>>
>> Documentation/git-branch.txt | 44
>> +++++++++++++++---------------------
>> 1 file changed, 18 insertions(+), 26 deletions(-)
>
> Thanks! I think this is a great improvement to this document.
Thank you, I'm glad that you like it. :)
>> diff --git a/Documentation/git-branch.txt
>> b/Documentation/git-branch.txt
>> index 0b0844293235..370ea43c0380 100644
>> --- a/Documentation/git-branch.txt
>> +++ b/Documentation/git-branch.txt
>> @@ -72,16 +72,6 @@ the remote-tracking branch. This behavior may be
>> changed via the global
>> overridden by using the `--track` and `--no-track` options, and
>> changed later using `git branch --set-upstream-to`.
>>
>> -With a `-m` or `-M` option, <oldbranch> will be renamed to
>> <newbranch>.
>> -If <oldbranch> had a corresponding reflog, it is renamed to match
>> -<newbranch>, and a reflog entry is created to remember the branch
>> -renaming. If <newbranch> exists, -M must be used to force the rename
>> -to happen.
>> -
>> -The `-c` and `-C` options have the exact same semantics as `-m` and
>> -`-M`, except instead of the branch being renamed, it will be copied
>> to a
>> -new name, along with its config and reflog.
>> -
>> With a `-d` or `-D` option, `<branchname>` will be deleted. You may
>> specify more than one branch for deletion. If the branch currently
>> has a reflog then the reflog will also be deleted.
>> @@ -128,18 +118,28 @@ Note that 'git branch -f <branchname>
>> [<start-point>]', even with '-f',
>> refuses to change an existing branch `<branchname>` that is checked
>> out
>> in another worktree linked to the same repository.
>>
>> --m::
>> ---move::
>> - Move/rename a branch, together with its config and reflog.
>> +-m [<oldbranch>] <newbranch>::
>> +--move [<oldbranch>] <newbranch>::
>> + Rename an existing branch <oldbranch>, which if not specified
>> defaults
>> + to the current branch, to <newbranch>. The configuration
>> variables
>
> Very minor nit: the prevailing style in this document appears to be to
> have a single space after the period, but it's definitely
> inconsistent. I don't see anything in Documentation/CodingGuidelines
> suggesting one way or the other, so don't consider this comment as
> blocking, just informational. If we want to achieve (eventual)
> consistency, we may want to use a single space after the period now.
In this case, I went with following the intersentence spacing used in
the surrounding text. Though, my brain and muscle memory are kind of
hardcoded to use double spacing, which may not be as prevalent as a
while
ago, but IMHO makes reading text rendered using a fixed-width font much
easier. I can adjust if needed, of course.
>> + for the <oldbranch> branch and its reflog are also renamed
>> appropriately
>> + to be used with <newbranch>. Renaming fails if branch
>> <newbranch>
>> + already exists, but you can use `-M` or `--move --force` to
>> overwrite
>> + the files in existing branch <newbranch> while renaming.
>>
>> --M::
>> +-M [<oldbranch>] <newbranch>::
>> Shortcut for `--move --force`.
>>
>> --c::
>> ---copy::
>> - Copy a branch, together with its config and reflog.
>> +-c [<oldbranch>] <newbranch>::
>> +--copy [<oldbranch>] <newbranch>::
>> + Copy an existing branch <oldbranch>, which if not specified
>> defaults
>> + to the current branch, to <newbranch>. The configuration
>> variables
>> + for the <oldbranch> branch and its reflog are also copied
>> appropriately
>> + to be used with <newbranch>. Copying fails if branch
>> <newbranch>
>> + already exists, but you can use `-C` or `--copy --force` to
>> overwrite
>> + the files in existing branch <newbranch> while copying.
>>
>> --C::
>> +-C [<oldbranch>] <newbranch>::
>> Shortcut for `--copy --force`.
>>
>> --color[=<when>]::
>> @@ -311,14 +311,6 @@ superproject's "origin/main", but tracks the
>> submodule's "origin/main".
>> given as a branch name, a commit-id, or a tag. If this
>> option is omitted, the current HEAD will be used instead.
>>
>> -<oldbranch>::
>> - The name of an existing branch. If this option is omitted,
>> - the name of the current branch will be used instead.
>> -
>> -<newbranch>::
>> - The new name for an existing branch. The same restrictions as
>> for
>> - <branchname> apply.
>> -
>> --sort=<key>::
>> Sort based on the key given. Prefix `-` to sort in descending
>> order of the value. You may use the --sort=<key> option
>>
^ permalink raw reply
* Re: [PATCH] branch: rework the descriptions of rename and copy operations
From: Kyle Lippincott @ 2024-02-15 22:56 UTC (permalink / raw)
To: Dragan Simic; +Cc: git
In-Reply-To: <e02b953f3320eff7d6ae2ecf684c8be2@manjaro.org>
On Thu, Feb 15, 2024 at 2:38 PM Dragan Simic <dsimic@manjaro.org> wrote:
>
> Hello Kyle,
>
> On 2024-02-15 23:31, Kyle Lippincott wrote:
> > On Thu, Feb 15, 2024 at 10:43 AM Dragan Simic <dsimic@manjaro.org>
> > wrote:
> >>
> >> Move the descriptions of the <oldbranch> and <newbranch> arguments to
> >> the
> >> descriptions of the branch rename and copy operations, where they
> >> naturally
> >> belong. Also, improve the descriptions of these two branch operations
> >> and,
> >> for completeness, describe the outcomes of forced operations.
> >>
> >> Describing the arguments together with their respective operations,
> >> instead
> >> of describing them separately in a rather unfortunate attempt to
> >> squeeze more
> >> meaning out of fewer words, flows much better and makes the
> >> git-branch(1)
> >> man page significantly more usable.
> >>
> >> The subsequent improvements shall continue this approach by either
> >> dissolving
> >> as many sentences from the "Description" section into the "Options"
> >> section,
> >> or by having those sentences converted into some kind of more readable
> >> and
> >> better flowing prose, as already discussed and outlined. [1][2]
> >>
> >> [1] https://lore.kernel.org/git/xmqqttmmlahf.fsf@gitster.g/T/#u
> >> [2] https://lore.kernel.org/git/xmqq8r4zln08.fsf@gitster.g/T/#u
> >>
> >> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
> >> ---
> >>
> >> Notes:
> >> This patch was originally named "branch: clarify <oldbranch> and
> >> <newbranch>
> >> terms further", submitted and discussed in another thread, [3] but
> >> the nature
> >> of the patch has changed, causing the patch subject to be adjusted
> >> to match.
> >>
> >> Consequently, this is effectively version 2 of the patch, which
> >> includes
> >> detailed feedback from Kyle and Junio, who suggested moving/adding
> >> the
> >> argument descriptions to their respective commands. This resulted
> >> in more
> >> significant changes to the contents of the git-branch(1) man page,
> >> in an
> >> attempt to make it more readable.
> >>
> >> [3]
> >> https://lore.kernel.org/git/e2eb777bca8ffeec42bdd684837d28dd52cfc9c3.1707136999.git.dsimic@manjaro.org/T/#u
> >>
> >> Documentation/git-branch.txt | 44
> >> +++++++++++++++---------------------
> >> 1 file changed, 18 insertions(+), 26 deletions(-)
> >
> > Thanks! I think this is a great improvement to this document.
>
> Thank you, I'm glad that you like it. :)
>
> >> diff --git a/Documentation/git-branch.txt
> >> b/Documentation/git-branch.txt
> >> index 0b0844293235..370ea43c0380 100644
> >> --- a/Documentation/git-branch.txt
> >> +++ b/Documentation/git-branch.txt
> >> @@ -72,16 +72,6 @@ the remote-tracking branch. This behavior may be
> >> changed via the global
> >> overridden by using the `--track` and `--no-track` options, and
> >> changed later using `git branch --set-upstream-to`.
> >>
> >> -With a `-m` or `-M` option, <oldbranch> will be renamed to
> >> <newbranch>.
> >> -If <oldbranch> had a corresponding reflog, it is renamed to match
> >> -<newbranch>, and a reflog entry is created to remember the branch
> >> -renaming. If <newbranch> exists, -M must be used to force the rename
> >> -to happen.
> >> -
> >> -The `-c` and `-C` options have the exact same semantics as `-m` and
> >> -`-M`, except instead of the branch being renamed, it will be copied
> >> to a
> >> -new name, along with its config and reflog.
> >> -
> >> With a `-d` or `-D` option, `<branchname>` will be deleted. You may
> >> specify more than one branch for deletion. If the branch currently
> >> has a reflog then the reflog will also be deleted.
> >> @@ -128,18 +118,28 @@ Note that 'git branch -f <branchname>
> >> [<start-point>]', even with '-f',
> >> refuses to change an existing branch `<branchname>` that is checked
> >> out
> >> in another worktree linked to the same repository.
> >>
> >> --m::
> >> ---move::
> >> - Move/rename a branch, together with its config and reflog.
> >> +-m [<oldbranch>] <newbranch>::
> >> +--move [<oldbranch>] <newbranch>::
> >> + Rename an existing branch <oldbranch>, which if not specified
> >> defaults
> >> + to the current branch, to <newbranch>. The configuration
> >> variables
> >
> > Very minor nit: the prevailing style in this document appears to be to
> > have a single space after the period, but it's definitely
> > inconsistent. I don't see anything in Documentation/CodingGuidelines
> > suggesting one way or the other, so don't consider this comment as
> > blocking, just informational. If we want to achieve (eventual)
> > consistency, we may want to use a single space after the period now.
>
> In this case, I went with following the intersentence spacing used in
> the surrounding text. Though, my brain and muscle memory are kind of
> hardcoded to use double spacing, which may not be as prevalent as a
> while
> ago, but IMHO makes reading text rendered using a fixed-width font much
> easier. I can adjust if needed, of course.
No need to adjust if I'm the only one mentioning it. I'm very new to
the project, so please don't take my style nits as any sort of
mandate, since I'm the one that's most likely to be miscalibrated. :)
>
> >> + for the <oldbranch> branch and its reflog are also renamed
> >> appropriately
> >> + to be used with <newbranch>. Renaming fails if branch
> >> <newbranch>
> >> + already exists, but you can use `-M` or `--move --force` to
> >> overwrite
> >> + the files in existing branch <newbranch> while renaming.
> >>
> >> --M::
> >> +-M [<oldbranch>] <newbranch>::
> >> Shortcut for `--move --force`.
> >>
> >> --c::
> >> ---copy::
> >> - Copy a branch, together with its config and reflog.
> >> +-c [<oldbranch>] <newbranch>::
> >> +--copy [<oldbranch>] <newbranch>::
> >> + Copy an existing branch <oldbranch>, which if not specified
> >> defaults
> >> + to the current branch, to <newbranch>. The configuration
> >> variables
> >> + for the <oldbranch> branch and its reflog are also copied
> >> appropriately
> >> + to be used with <newbranch>. Copying fails if branch
> >> <newbranch>
> >> + already exists, but you can use `-C` or `--copy --force` to
> >> overwrite
> >> + the files in existing branch <newbranch> while copying.
> >>
> >> --C::
> >> +-C [<oldbranch>] <newbranch>::
> >> Shortcut for `--copy --force`.
> >>
> >> --color[=<when>]::
> >> @@ -311,14 +311,6 @@ superproject's "origin/main", but tracks the
> >> submodule's "origin/main".
> >> given as a branch name, a commit-id, or a tag. If this
> >> option is omitted, the current HEAD will be used instead.
> >>
> >> -<oldbranch>::
> >> - The name of an existing branch. If this option is omitted,
> >> - the name of the current branch will be used instead.
> >> -
> >> -<newbranch>::
> >> - The new name for an existing branch. The same restrictions as
> >> for
> >> - <branchname> apply.
> >> -
> >> --sort=<key>::
> >> Sort based on the key given. Prefix `-` to sort in descending
> >> order of the value. You may use the --sort=<key> option
> >>
^ permalink raw reply
* Re: [PATCH] branch: rework the descriptions of rename and copy operations
From: Dragan Simic @ 2024-02-15 23:09 UTC (permalink / raw)
To: Kyle Lippincott; +Cc: git
In-Reply-To: <CAO_smVizZ6K-tSSubr7Pd95zn42GAHrgeeSQ7ZWmKdPH7gMy3w@mail.gmail.com>
On 2024-02-15 23:56, Kyle Lippincott wrote:
> On Thu, Feb 15, 2024 at 2:38 PM Dragan Simic <dsimic@manjaro.org>
> wrote:
>> On 2024-02-15 23:31, Kyle Lippincott wrote:
>> > Very minor nit: the prevailing style in this document appears to be to
>> > have a single space after the period, but it's definitely
>> > inconsistent. I don't see anything in Documentation/CodingGuidelines
>> > suggesting one way or the other, so don't consider this comment as
>> > blocking, just informational. If we want to achieve (eventual)
>> > consistency, we may want to use a single space after the period now.
>>
>> In this case, I went with following the intersentence spacing used in
>> the surrounding text. Though, my brain and muscle memory are kind of
>> hardcoded to use double spacing, which may not be as prevalent as a
>> while
>> ago, but IMHO makes reading text rendered using a fixed-width font
>> much
>> easier. I can adjust if needed, of course.
>
> No need to adjust if I'm the only one mentioning it. I'm very new to
> the project, so please don't take my style nits as any sort of
> mandate, since I'm the one that's most likely to be miscalibrated. :)
Basically, when it comes to the intersentence spacing, it seems that
double spacing has lost its popularity over time. It originates from
the old times when mechanical typewriters were used, which most people
probably don't even remember these days. :)
^ permalink raw reply
* Re: [PATCH] use C99 declaration of variable in for() loop
From: brian m. carlson @ 2024-02-15 23:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Elia Pinto, git
In-Reply-To: <xmqqcysxskd9.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 1622 bytes --]
On 2024-02-15 at 17:33:22, Junio C Hamano wrote:
> Elia Pinto <gitter.spiros@gmail.com> writes:
>
> > With the exception of cbtree.c, which would need initial
> > reworking to remove the usage of goto, it expands the
> > use of variable scope reduction in for loops as
> > permitted by the C99 standard, which was first introduced
> > in the git codebase with commit 44ba10d6.
>
> Thanks, but ...
>
> Our test balloon may have proven that nobody will be inconvenienced,
> and it does mean we can be liberal using it when we add new code or
> update existing loops "while at it", but I personally do not think
> such a code churn is very welcome.
I will also say that sending one giant patch for this may be a bit hard
to review. While I will defer to Junio's opinion as the maintainer, I
would be more inclined to review this kind of series if it came in in
smaller patches, a few at a time, in which case I would find it a
welcome improvement.
Since my time to work on Git is relatively limited, having, say, a
five-patch series where we each update a single file would let me review
these changes in a relatively short amount of time, which I would be
more likely to be able to find time for. Looking at the large patch,
I'd be worried that I wouldn't be able to get through the entire thing
in one sitting.
Of course, if you bring in a nice Coccinelle patch for it, then that may
make a longer (but still one-file-per-commit) series more viable, since
it will help reviewers have more confidence in your change.
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply
* [PATCH] cmake: let `test-tool` run the unit tests, too
From: Johannes Schindelin via GitGitGadget @ 2024-02-15 23:15 UTC (permalink / raw)
To: git; +Cc: Josh Steadmon, Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The `test-tool` recently learned to run the unit tests. To this end, it
needs to link with `test-lib.c`, which was done in the `Makefile`, and
this patch does it in the CMake definition, too.
This is a companion of 44400f58407e (t0080: turn t-basic unit test into
a helper, 2024-02-02).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
cmake: let test-tool run the unit tests, too
The test-tool recently learned to run the unit tests. To this end, it
needs to link with test-lib.c, which was done in the Makefile, and this
patch does it in the CMake definition, too.
This is a companion of 44400f58407e (t0080: turn t-basic unit test into
a helper, 2024-02-02), and is based on js/unit-test-suite-runner.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1666%2Fgit-for-windows%2Fjs%2Funit-test-suite-runner-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1666/git-for-windows/js/unit-test-suite-runner-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1666
contrib/buildsystems/CMakeLists.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 804629c525b..2f9c33585c6 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -1005,10 +1005,11 @@ endforeach()
#test-tool
parse_makefile_for_sources(test-tool_SOURCES "TEST_BUILTINS_OBJS")
+add_library(test-lib OBJECT ${CMAKE_SOURCE_DIR}/t/unit-tests/test-lib.c)
list(TRANSFORM test-tool_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/t/helper/")
add_executable(test-tool ${CMAKE_SOURCE_DIR}/t/helper/test-tool.c ${test-tool_SOURCES} ${test-reftable_SOURCES})
-target_link_libraries(test-tool common-main)
+target_link_libraries(test-tool test-lib common-main)
set_target_properties(test-fake-ssh test-tool
PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/t/helper)
base-commit: b3b269c2d8931642c4b9f03b9ce9e81c20995eb8
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH] branch: rework the descriptions of rename and copy operations
From: Rubén Justo @ 2024-02-15 23:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Dragan Simic, git
In-Reply-To: <xmqq8r3lnzp0.fsf@gitster.g>
On 15-feb-2024 14:13:31, Junio C Hamano wrote:
> Rubén Justo <rjusto@gmail.com> writes:
>
> > On 15-feb-2024 19:42:32, Dragan Simic wrote:
> >
> >> Move the descriptions of the <oldbranch> and <newbranch> arguments to the
> >> descriptions of the branch rename and copy operations, where they naturally
> >> belong.
> >
> > Thank you Dragan for working on this.
> >
> > Let me chime in just to say that maybe another terms could be considered
> > here; like: "<branchname>" and "<newbranchname>" (maybe too long...) or
> > so.
> >
> > I have no problem with the current terms, but "<branchname>" can be a
> > sensible choice here as it is already being used for other commands
> > where, and this may help overall, the consideration: "if ommited, the
> > current branch is considered" also applies.
>
> Actually, we should go in the opposite direction. When the use of
> names are localized in a narrower context, they can be shortened
> without losing clarity.
I did not mean to have longer terms, sorry for that.
I was thinking more in the synopsis:
'git branch' (--set-upstream-to=<upstream> | -u <upstream>) [<branchname>]
'git branch' --unset-upstream [<branchname>]
'git branch' (-m | -M) [<branchname>] <new>
'git branch' (-c | -C) [<branchname>] <new>
'git branch' (-d | -D) [-r] <branchname>...
'git branch' --edit-description [<branchname>]
To have more uniformity in the terms, which can be beneficial to the
user.
We don't say that "--edit-description" defaults to the current branch;
It is assumed. Perhaps we can take advantage of that assumption in
-m|-c too.
Of course, there is no need (perhaps counterproductive) to say "branch"
if the context makes it clear it is referring to a branch.
> For example:
>
> -m [<old>] <new>::
> rename the <old> branch (defaults to the current one) to
> <new>.
>
> is just as clear as the same description with <oldbranch> and
> <newbranch>. With the original text without any of the suggested
> changes, <oldbranch> and <newbranch> appeared very far from the
> context they are used in (i.e. the description for -m and -c), and
> it may have helped readers to tell that these are names of branches.
> But if the context is clear that we are talking about "renaming"
> branches, there is not as much added benefit to say "branch" in
> these names as in the current text.
^ permalink raw reply
* Re: [PATCH] branch: rework the descriptions of rename and copy operations
From: Rubén Justo @ 2024-02-15 23:38 UTC (permalink / raw)
To: Dragan Simic; +Cc: git
In-Reply-To: <6caad50252bac7f75da8de7e3728a45e@manjaro.org>
On 15-feb-2024 23:27:59, Dragan Simic wrote:
> Hello Ruben and Junio,
>
> On 2024-02-15 22:52, Rubén Justo wrote:
> > On 15-feb-2024 19:42:32, Dragan Simic wrote:
> >
> > > Move the descriptions of the <oldbranch> and <newbranch> arguments
> > > to the
> > > descriptions of the branch rename and copy operations, where they
> > > naturally
> > > belong.
> >
> > Thank you Dragan for working on this.
>
> Thank you, and everyone else, for the reviews and suggestions.
>
> > Let me chime in just to say that maybe another terms could be considered
> > here; like: "<branchname>" and "<newbranchname>" (maybe too long...) or
> > so.
> >
> > I have no problem with the current terms, but "<branchname>" can be a
> > sensible choice here as it is already being used for other commands
> > where, and this may help overall, the consideration: "if ommited, the
> > current branch is considered" also applies.
>
> Actually, I'd agree with Junio's reply that suggested using even
> shorter terms.
Me too :-)
> Just like "<oldbranch>" and "<newbranch>" can safely
> be shortened to "<old>" and "<new>", respectively, "<branchname>"
> can also be shortened to "<name>".
>
> It's all about the context, which is improved by moving the descriptions
> of the arguments closer to the descriptions of the commands.
Your series is an improvement in that respect. Thank you.
>
> Though, I'd prefer that we keep "<oldbranch>" and "<newbranch>" (and
> "<branchname>") for now, for the sake of consistency, and I'd get them
> shortened in the future patches.
Nice!
^ permalink raw reply
* Re: [PATCH] use C99 declaration of variable in for() loop
From: Junio C Hamano @ 2024-02-15 23:43 UTC (permalink / raw)
To: brian m. carlson; +Cc: Elia Pinto, git
In-Reply-To: <Zc6abO6RV9RwpABR@tapette.crustytoothpaste.net>
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> I will also say that sending one giant patch for this may be a bit hard
> to review. While I will defer to Junio's opinion as the maintainer, I
> would be more inclined to review this kind of series if it came in in
> smaller patches, a few at a time, in which case I would find it a
> welcome improvement.
True. As to the specific topic of using "for (int i = 0; ...)",
it is tedious to review for mistakes and 17000+ lines of patch is
not a way to do so. I do not think I would be able to spot a change
in behaviour caused by a hunk like this
int i = 3;
... after some operations ...
- for (i = 0; i < 5; i++)
+ for (int i = 0; i < 5; i++)
if (condition_on_i(i))
break;
... after some operations ...
return i;
after scanning similar changes for 1000+ times in a single huge
patch.
^ 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