core.commentChar used to be limited to a single byte, but has been
updated to allow an arbitrary multi-byte sequence.
* jk/core-comment-string:
config: add core.commentString
config: allow multi-byte core.commentChar
environment: drop comment_line_char compatibility macro
wt-status: drop custom comment-char stringification
sequencer: handle multi-byte comment characters when writing todo list
find multi-byte comment chars in unterminated buffers
find multi-byte comment chars in NUL-terminated strings
prefer comment_line_str to comment_line_char for printing
strbuf: accept a comment string for strbuf_add_commented_lines()
strbuf: accept a comment string for strbuf_commented_addf()
strbuf: accept a comment string for strbuf_stripspace()
environment: store comment_line_char as a string
strbuf: avoid shadowing global comment_line_char name
commit: refactor base-case of adjust_comment_line_char()
strbuf: avoid static variables in strbuf_add_commented_lines()
strbuf: simplify comment-handling in add_lines() helper
config: forbid newline as core.commentChar
The "--pretty=<shortHand>" option of the commands in the "git log"
family, defined as "[pretty] shortHand = <expansion>" should have
been looked up case insensitively, but was not, which has been
corrected.
* bl/pretty-shorthand-config-fix:
pretty: find pretty formats case-insensitively
pretty: update tests to use `test_config`
"git config" learned "--comment=<message>" option to leave a
comment immediately after the "variable = value" on the same line
in the configuration file.
* rs/config-comment:
config: allow tweaking whitespace between value and comment
config: fix --comment formatting
config: add --comment option to add a comment
The README now gives a hint on running individual tests in the "t/"
directory with "make t<num>-*.sh t<num>-*.sh".
* pb/test-scripts-are-build-targets:
t/README: mention test files are make targets
Documentation updates.
* ds/grep-doc-updates:
grep docs: describe --no-index further and improve formatting a bit
grep docs: describe --recurse-submodules further and improve formatting a bit
Introduce an experimental protocol for contributors to propose the
topic description to be used in the "What's cooking" report, the
merge commit message for the topic, and in the release notes and
document it in the SubmittingPatches document.
* jc/release-notes-entry-experiment:
SubmittingPatches: release-notes entry experiment
The implementation and documentation of "object-format" option
exchange between the Git itself and its remote helpers did not
quite match.
* jk/remote-helper-object-format-option-fix:
transport-helper: send "true" value for object-format option
transport-helper: drop "object-format <algo>" option
transport-helper: use write helpers more consistently
"git apply" failed to extract the filename the patch applied to,
when the change was about an empty file created in or deleted from
a directory whose name ends with a SP, which has been corrected.
* jc/apply-parse-diff-git-header-names-fix:
apply: parse names out of "diff --git" more carefully
The core.commentChar code recently learned to accept more than a
single ASCII character. But using it is annoying with multiple versions
of Git, since older ones will reject it outright:
$ git.v2.44.0 -c core.commentchar=foo stripspace -s
error: core.commentChar should only be one ASCII character
fatal: unable to parse 'core.commentchar' from command-line config
Let's add an alias core.commentString. That's arguably a better name
anyway, since we now can handle strings, and it makes it possible to
have a config that works reasonably with both old and new versions of
Git (see the example in the documentation).
This is strictly an alias, so there's not much point in adding duplicate
tests; I added a single one to t0030 that exercises the alias code.
Note also that the error messages for invalid values will now show the
variable the config parser handed us, and thus will be normalized to
lowercase (rather than camelcase). A few tests in t0030 are adjusted to
match.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "What's cooking" report lists the topics in flight, with a short
paragraph descibing what they are about.
Once written, the description is automatically picked up from the
"What's cooking" report and used in the commit log message of the
merge commit when the topic is merged into integration branches.
These commit log messges of the merge commits are then propagated to
the release notes.
It has been the maintainer's task to prepare these entries in the
"What's cooking" report. Even though the original author of a topic
may be in the best position to write the initial description of a
topic, we so far lacked a formal channel for the author to suggest
what description to use. The usual procedure has been for the
author to see the topic described in "What's cooking" report, and
then either complain about inaccurate explanation and/or offer a
rewrite.
Let's try an experiment to optionally let the author propose the one
paragraph description when the topic is submitted. Pick the cover
letter as the logical place to do so, and describe an experimental
workflow in the SubmittingPatches document.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix the way recently added tests interpolate variables defined
outside them, and document the best practice to help future
developers.
* ps/t7800-variable-interpolation-fix:
t/README: document how to loop around test cases
t7800: use single quotes for test bodies
t7800: improve test descriptions with empty arguments
A unit test for reftable code tried to enumerate all files in a
directory after reftable operations and expected to see nothing but
the files it wanted to leave there, but was fooled by .nfs* cruft
files left, which has been corrected.
* ps/reftable-unit-test-nfs-workaround:
reftable: fix tests being broken by NFS' delete-after-close semantics
Hints that suggest what to do after resolving conflicts can now be
squelched by disabling advice.mergeConflict.
Acked-by: Phillip Wood <phillip.wood123@gmail.com>
cf. <e040c631-42d9-4501-a7b8-046f8dac6309@gmail.com>
* pb/advice-merge-conflict:
builtin/am: allow disabling conflict advice
sequencer: allow disabling conflict advice
"git config" corrupted literal HT characters written in the
configuration file as part of a value, which has been corrected.
* ds/config-internal-whitespace-fix:
config.txt: describe handling of whitespace further
t1300: add more tests for whitespace and inline comments
config: really keep value-internal whitespace verbatim
config: minor addition of whitespace
The documentation for "%(trailers[:options])" placeholder in the
"--pretty" option of commands in the "git log" family has been
updated.
* bl/doc-key-val-sep-fix:
docs: adjust trailer `separator` and `key_value_separator` language
docs: correct trailer `key_value_separator` description
A few typoes in "git config --help" have been corrected.
* bl/doc-config-fixes:
docs: fix typo in git-config `--default`
docs: clarify file options in git-config `--edit`
"git bugreport --no-suffix" was not supported and instead
segfaulted, which has been corrected.
* js/bugreport-no-suffix-fix:
bugreport.c: fix a crash in `git bugreport` with `--no-suffix` option
Improve the description of --no-index, to make it more clear to the users
what this option actually does under the hood, and what's its purpose.
Describe the dependency between --no-index and either of the --cached and
--untracked options, which cannot be used together.
As part of that, shuffle a couple of the options, to make the documentation
flow a bit better, because it makes more sense to describe first the options
that have something in common, and to after that describe an option that does
something differently. In more detail, --cached and --untracked both leave
git-grep(1) in the usual state, in which it treats the directory as a local
git repository, unlike --no-index that makes git-grep(1) treat the directory
not as a git repository.
While there, improve the descriptions of grep worker threads a bit, to give
them better context. Adjust the language a bit, to avoid addressing the
reader directly, which is in general preferred in technical documentation,
because it eliminates the possible element of persuading the user to do
something. In other words, we should be telling the user what our software
can do, instead of telling the user what to do.
Also perform some minor formatting improvements, to make it clear it's the
git commands, command parameters, and configuration option names.
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Clarify that --recurse-submodules cannot be used together with --untracked,
and improve the formatting in a couple of places, to make it visually clear
that those are the commands or the names of configuration options.
While there, change a couple of "<tree>" placeholders to "_<tree>_", to help
with an ongoing translation improvement effort. [1]
[1] https://lore.kernel.org/git/CAPig+cQc8W4JOpB+TMP=czketU1U7wcY_x9bsP5T=3-XjGLhRQ@mail.gmail.com/
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
User-defined pretty formats are stored in config, which is meant to use
case-insensitive matching for names as noted in config.txt's 'Syntax'
section:
All the other lines [...] are recognized as setting variables, in
the form 'name = value' [...]. The variable names are
case-insensitive, [...].
When a user specifies one of their format aliases with an uppercase in
it, however, it is not found.
$ git config pretty.testAlias %h
$ git config --list | grep pretty
pretty.testalias=%h
$ git log --format=testAlias -1
fatal: invalid --pretty format: testAlias
$ git log --format=testalias -1
3c2a3fdc38
This is true whether the name in the config file uses any uppercase
characters or not.
Use case-insensitive comparisons when identifying format aliases.
Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Brian Lyles <brianmlyles@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
These tests use raw `git config` calls, which is an older style that can
cause config to bleed between tests if not manually unset. `test_config`
ensures that config is unset at the end of each test automatically.
`test_config` is chosen over `git -c` since `test_config` still ends up
calling `git config` which seems slightly more realistic to how pretty
formats would be defined normally.
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Brian Lyles <brianmlyles@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Build the pack base name argument directly using strvec_pushf() instead
of with an intermediate strbuf. This is shorter, simpler and avoids the
need for explicit cleanup.
Signed-off-by: René Scharfe <l.s.r@web.de>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since 23fc63bf8f (make tests ignorable with "make -i", 2005-11-08), each
test file defines a target in the test Makefile, such that one can
invoke:
make *checkout*
to run all tests with 'checkout' in their filename. This is useful to
run a subset of tests when you have a good idea of what part of the code
is touched by the changes your are testing.
Document that in t/README to help new (or more seasoned) contributors
that might not be aware.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Report unknown format elements and missing closing parentheses with
consistent and translated messages by calling strbuf_expand_bad_format()
at the very end of the combined if/else chain of expand_format() and
expand_atom().
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Extract a function for reporting placeholders that are not enclosed in a
parenthesis or are unknown. This reduces the number of strings to
translate and improves consistency across commands. Call it at the end
of the if/else chain, after exhausting all accepted possibilities.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Imagine you want to grep for (. Easy:
$ git grep '('
fatal: unmatched parenthesis
uhoh. This is plainly wrong. Unless you know specifically that
(a) git grep has expression groups and '(' ... ')' are used for them.
(b) you can use -e '(' to explicitly say '(' is what you are looking
for, not the beginning of a group.
Similarly,
$ git grep ')'
fatal: incomplete pattern expression: )
is somehow worse. ")" is a complete regular expression pattern.
Of course, the error wants to say "group" here.
In this case it is also not "incomplete", it is unmatched.
Make them say
$ ./git grep '('
fatal: unmatched ( for expression group
$ ./git grep ')'
fatal: incomplete pattern expression group: )
which are clearer in indicating that it is not the expression that
is wrong (since no pattern had been parsed at all), but rather that
it is been misconstrued as a grouping operator.
Link: https://bugs.debian.org/1051205
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Code clean-up in the "git log" machinery that implements custom log
message formatting.
* jk/pretty-subject-cleanup:
format-patch: fix leak of empty header string
format-patch: simplify after-subject MIME header handling
format-patch: return an allocated string from log_write_email_headers()
log: do not set up extra_headers for non-email formats
pretty: drop print_email_subject flag
pretty: split oneline and email subject printing
shortlog: stop setting pp.print_email_subject
"git checkout --conflict=bad" reported a bad conflictStyle as if it
were given to a configuration variable; it has been corrected to
report that the command line option is bad.
* pw/checkout-conflict-errorfix:
checkout: fix interaction between --conflict and --merge
checkout: cleanup --conflict=<style> parsing
merge options: add a conflict style member
merge-ll: introduce LL_MERGE_OPTIONS_INIT
xdiff-interface: refactor parsing of merge.conflictstyle
In the run_am() function, we set up a child_process struct to run
"git-am", allocating memory for its args and env strvecs. These are
normally cleaned up when we call run_command(). But if we encounter
certain errors, we exit the function early and try to clean up ourselves
by clearing the am.args field. This leaks the "env" strvec.
We should use child_process_clear() instead, which covers both. And more
importantly, it future proofs us against the struct ever growing more
allocated fields.
These are unlikely errors to happen in practice, so they don't actually
trigger the leak sanitizer in the tests. But we can add a new test which
does exercise one of the paths (and fails SANITIZE=leak without this
patch).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The log_write_email_headers() function recently learned to return the
"extra_headers_p" variable to the caller as an allocated string. We
start by copying rev_info.extra_headers into a strbuf, and then detach
the strbuf at the end of the function. If there are no extra headers, we
leave the strbuf empty. Likewise, if there are no headers to return, we
pass back NULL.
This misses a corner case which can cause a leak. The "do we have any
headers to copy" check is done by looking for a NULL opt->extra_headers.
But the "do we have a non-empty string to return" check is done by
checking the length of the strbuf. That means if opt->extra_headers is
the empty string, we'll "copy" it into the strbuf, triggering an
allocation, but then leak the buffer when we return NULL from the
function.
We can solve this in one of two ways:
1. Rather than checking headers->len at the end, we could check
headers->alloc to see if we allocated anything. That retains the
original behavior before the recent change, where an empty
extra_headers string is "passed through" to the caller. In practice
this doesn't matter, though (the code which eventually looks at the
result treats NULL or the empty string the same).
2. Only bother copying a non-empty string into the strbuf. This has
the added bonus of avoiding a pointless allocation.
Arguably strbuf_addstr() could do this optimization itself, though
it may be slightly dangerous to do so (some existing callers may
not get a fresh allocation when they expect to). In theory callers
are all supposed to use strbuf_detach() in such a case, but there's
no guarantee that this is the case.
This patch uses option 2. Without it, building with SANITIZE=leak shows
many errors in t4021 and elsewhere.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In some cases it makes sense to loop around test cases so that we can
execute the same test with slightly different arguments. There are some
gotchas around quoting here though that are easy to miss and that may
lead to easy-to-miss errors and portability issues.
Document the proper way to do this in "t/README".
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In eb84c8b6ce (git-difftool--helper: honor `--trust-exit-code` with
`--dir-diff`, 2024-02-20) we have started to loop around some of the
tests in t7800 so that they are reexecuted with slightly different
arguments. As part of that refactoring the quoting of test bodies was
changed from single quotes (') to double quotes (") so that the value of
the loop variable is accessible to the body.
As the test body is later on passed to eval this change was not required
though. Let's revert it back to use single quotes as usual in our tests.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some of the tests in t7800 are executed repeatedly in a loop with
different arguments. To distinguish these tests, the value of that
variable is rendered into the test title. But given that one of the
values is the empty string, it results in a somewhat awkward test name:
difftool ignores exit code
Improve this by printing "without options" in case the value is empty.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Make it more clear what the whitespace characters are in the context of git
configuration files, and significantly improve the description of the leading
and trailing whitespace handling, especially how it works out together with
the presence of inline comments.
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a handful of additional tests, to improve the coverage of the handling
of configuration file entries whose values contain internal whitespace,
leading and/or trailing whitespace, which may or may not be enclosed within
quotation marks, or which contain an additional inline comment.
At the same time, rework one already existing whitespace-related test a bit,
to ensure its consistency with the newly added tests. This change introduced
no functional changes to the already existing test.
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix a bug in function parse_value() that prevented whitespace characters
(i.e. spaces and horizontal tabs) found inside configuration option values
from being parsed and returned in their original form. The bug caused any
number of consecutive whitespace characters to be wrongly "squashed" into
the same number of space characters.
This bug was introduced back in July 2009, in commit ebdaae372b ("config:
Keep inner whitespace verbatim").
Further investigation showed that setting a configuration value, by invoking
git-config(1), converts value-internal horizontal tabs into "\t" escape
sequences, which the buggy value-parsing logic in function parse_value()
didn't "squash" into spaces. That's why the test included in the ebdaae37
commit passed, which presumably made the bug remain undetected for this long.
On the other hand, value-internal literal horizontal tab characters, found in
a configuration file edited by hand, do get "squashed" by the value-parsing
logic, so the right choice was to fix this bug by making the value-internal
whitespace characters preserved verbatim.
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In general, binary operators should be enclosed in a pair of leading and
trailing space (SP) characters. Thus, clean up one spotted expression that
for some reason had a "bunched up" operator.
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano
Jeff King
Dragan Simic
Brian Lyles
René Scharfe
Philippe Blain
Ahelenia Ziemiańska
Patrick Steinhardt