summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2024-03-15format_trailer_info(): append newline for non-trailer linesLinus Arver3-22/+11
This wraps up the preparatory refactors to unify the trailer formatters. Two patches ago we made format_trailer_info() use trailer_item objects instead of the "trailers" string array. The strings in the array include trailing newlines, because the string array is split up with trailer_lines = strbuf_split_buf(str + trailer_block_start, end_of_log_message - trailer_block_start, '\n', 0); in trailer_info_get() and strbuf_split_buf() includes the terminator (in this case the newline character '\n') for each split-up substring. And before we made the transition to use trailer_item objects for it, format_trailer_info() called parse_trailer() (which trims newlines) for trailer lines but did _not_ call parse_trailer() for non-trailer lines. So for trailer lines it had to add back the trimmed newline like this if (!opts->separator) strbuf_addch(out, '\n'); But for non-trailer lines it didn't have to add back the newline because it could just reuse same string in the "trailers" string array (which again, already included the trailing newline). Now that format_trailer_info() uses trailer_item objects for all cases, it can't rely on "trailers" string array anymore. And so it must be taught to add a newline back when printing non-trailer lines, just like it already does for trailer lines. Do so now. The test suite can pass again without the need to hide failures with *_failure, so flip the affected test cases back to *_success. Now, format_trailer_info() is in better shape to supersede format_trailers(), which we'll do in the next commit. Signed-off-by: Linus Arver <linusa@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-15format_trailer_info(): drop redundant unfold_value()Linus Arver1-3/+0
This is another preparatory refactor to unify the trailer formatters. In the last patch we made format_trailer_info() use trailer_item objects instead of the "trailers" string array. This means that the call to unfold_value() here is redundant because the trailer_item objects are already unfolded in parse_trailers() which is a dependency of our caller, format_trailers_from_commit(). Remove the redundant call. Signed-off-by: Linus Arver <linusa@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-15format_trailer_info(): use trailer_item objectsLinus Arver3-19/+30
This is another preparatory refactor to unify the trailer formatters. Make format_trailer_info() operate on trailer_item objects, not the raw string array. We will continue to make improvements, culminating in the renaming of format_trailer_info() to format_trailers(), at which point the unification of these formatters will be complete. In order to avoid breaking t4205 and t6300, flip *_success to *_failure in the affected test cases. Add a temporary "test_trailer_option_expect_failure" wrapper which we will use along with "test_expect_failure" in the next commit to avoid breaking tests. When the dust settles with the refactors a few more commits later, we will drop the use of *_failure to make the tests truly pass again. When the preparatory refactors are complete, we'll be able to drop the use of *_failure that we introduce here. Signed-off-by: Linus Arver <linusa@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-15t5300: fix test_with_bad_commit()John Cai1-1/+1
0f8edf7317 (index-pack: --fsck-objects to take an optional argument for fsck msgs, 2024-02-01) added a test function test_with_bad_commit() that contained two bugs. test_expect_fail was used instead of test_must_fail, and a && was not included at the end of the line. Fix these two issues in the test. Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-15diff: add diff.srcPrefix and diff.dstPrefix configuration variablesPeter Hutterer4-4/+56
Allow the default prefixes "a/" and "b/" to be tweaked by the diff.srcPrefix and diff.dstPrefix configuration variables. Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-14The seventh batchJunio C Hamano1-0/+32
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-14Merge branch 'rj/complete-worktree-paths-fix'Junio C Hamano2-1/+24
The logic to complete the command line arguments to "git worktree" subcommand (in contrib/) has been updated to correctly honor things like "git -C dir" etc. * rj/complete-worktree-paths-fix: completion: fix __git_complete_worktree_paths
2024-03-14Merge branch 'rj/complete-reflog'Junio C Hamano2-12/+82
The command line completion script (in contrib/) learned to complete "git reflog" better. * rj/complete-reflog: completion: reflog subcommands and options completion: factor out __git_resolve_builtins completion: introduce __git_find_subcommand completion: reflog show <log-options> completion: reflog with implicit "show"
2024-03-14Merge branch 'jc/test-i18ngrep'Junio C Hamano1-2/+1
With release 2.44 we got rid of all uses of test_i18ngrep and there is no in-flight topic that adds a new use of it. Make a call to test_i18ngrep a hard failure, so that we can remove it at the end of this release cycle. * jc/test-i18ngrep: test_i18ngrep: hard deprecate and forbid its use
2024-03-14Merge branch 'gt/core-bare-in-templates'Junio C Hamano3-49/+8
Code simplification. * gt/core-bare-in-templates: setup: remove unnecessary variable
2024-03-14Merge branch 'la/trailer-api'Junio C Hamano7-147/+204
Trailer API updates. Acked-by: Christian Couder <christian.couder@gmail.com> cf. <CAP8UFD1Zd+9q0z1JmfOf60S2vn5-sD3SafDvAJUzRFwHJKcb8A@mail.gmail.com> * la/trailer-api: format_trailers_from_commit(): indirectly call trailer_info_get() format_trailer_info(): move "fast path" to caller format_trailers(): use strbuf instead of FILE trailer_info_get(): reorder parameters trailer: move interpret_trailers() to interpret-trailers.c trailer: reorder format_trailers_from_commit() parameters trailer: rename functions to use 'trailer' shortlog: add test for de-duplicating folded trailers trailer: free trailer_info _after_ all related usage
2024-03-14Merge branch 'kh/doc-commentchar-is-a-byte'Junio C Hamano1-1/+1
The "core.commentChar" configuration variable only allows an ASCII character, which was not clearly documented, which has been corrected. * kh/doc-commentchar-is-a-byte: config: document `core.commentChar` as ASCII-only
2024-03-14Merge branch 'jh/fsmonitor-icase-corner-case-fix'Junio C Hamano6-56/+522
FSMonitor client code was confused when FSEvents were given in a different case on a case-insensitive filesystem, which has been corrected. Acked-by: Patrick Steinhardt <ps@pks.im> cf. <ZehofMaSZyUq8S1N@tanuki> * jh/fsmonitor-icase-corner-case-fix: fsmonitor: support case-insensitive events fsmonitor: refactor bit invalidation in refresh callback fsmonitor: trace the new invalidated cache-entry count fsmonitor: return invalidated cache-entry count on non-directory event fsmonitor: remove custom loop from non-directory path handler fsmonitor: return invalidated cache-entry count on directory event fsmonitor: move untracked-cache invalidation into helper functions fsmonitor: refactor untracked-cache invalidation dir: create untracked_cache_invalidate_trimmed_path() fsmonitor: refactor refresh callback for non-directory events fsmonitor: clarify handling of directory events in callback helper fsmonitor: refactor refresh callback on directory events t7527: add case-insensitve test for FSMonitor name-hash: add index_dir_find()
2024-03-14Merge branch 'ps/reftable-iteration-perf-part2'Junio C Hamano14-181/+175
The code to iterate over refs with the reftable backend has seen some optimization. * ps/reftable-iteration-perf-part2: refs/reftable: precompute prefix length reftable: allow inlining of a few functions reftable/record: decode keys in place reftable/record: reuse refname when copying reftable/record: reuse refname when decoding reftable/merged: avoid duplicate pqueue emptiness check reftable/merged: circumvent pqueue with single subiter reftable/merged: handle subiter cleanup on close only reftable/merged: remove unnecessary null check for subiters reftable/merged: make subiters own their records reftable/merged: advance subiter on subsequent iteration reftable/merged: make `merged_iter` structure private reftable/pq: use `size_t` to track iterator index
2024-03-14Merge branch 'so/clean-dry-run-without-force'Junio C Hamano4-18/+18
The implementation in "git clean" that makes "-n" and "-i" ignore clean.requireForce has been simplified, together with the documentation. * so/clean-dry-run-without-force: clean: further clean-up of implementation around "--force" clean: improve -n and -f implementation and documentation
2024-03-14checkout: plug some leaks in git-restoreRubén Justo5-30/+25
In git-restore we need to free the pathspec and pathspec_from_file values from the struct checkout_opts. A simple fix could be to free them in cmd_restore, after the call to checkout_main returns, like we are doing [1][2] in the sibling function cmd_checkout. However, we can do even better. We have git-switch and git-restore, both of them spin-offs[3][4] of git-checkout. All three are implemented as thin wrappers around checkout_main. Considering this, it makes a lot of sense to do the cleanup closer to checkout_main. Move the cleanups, including the new_branch_info variable, to checkout_main. As a consequence, mark: t2070, t2071, t2072 and t6418 as leak-free. [1] 9081a421a6 (checkout: fix "branch info" memory leaks, 2021-11-16) [2] 7ce4088ab7 (parse-options: consistently allocate memory in fix_filename(), 2023-03-04) [3] d787d311db (checkout: split part of it to new command 'switch', 2019-03-29) [4] 46e91b663b (checkout: split part of it to new command 'restore', 2019-04-25) Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-14checkout: fix interaction between --conflict and --mergePhillip Wood2-3/+67
When using "git checkout" to recreate merge conflicts or merge uncommitted changes when switching branch "--conflict" sensibly implies "--merge". Unfortunately the way this is implemented means that "git checkout --conflict=diff3 --no-merge" implies "--merge" violating the usual last-one-wins rule. Fix this by only overriding the value of opts->merge if "--conflicts" comes after "--no-merge" or "-[-no]-merge" is not given on the command line. The behavior of "git checkout --merge --no-conflict" is unchanged and will still merge on the basis that the "-[-no]-conflict" options are primarily intended to affect the conflict style and so "--no-conflict" should cancel a previous "--conflict" but not override "--merge". Of the four new tests the second one tests the behavior change introduced by this commit, the other three check that this commit does not regress the existing behavior. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-14checkout: cleanup --conflict=<style> parsingPhillip Wood2-19/+38
Passing an invalid conflict style name such as "--conflict=bad" gives the error message error: unknown style 'bad' given for 'merge.conflictstyle' which is unfortunate as it talks about a config setting rather than the option given on the command line. This happens because the implementation calls git_xmerge_config() to set the conflict style using the value given on the command line. Use the newly added parse_conflict_style_name() instead and pass the value down the call chain to override the config setting. This also means we can avoid setting up a struct config_context required for calling git_xmerge_config(). The option is now parsed in a callback to avoid having to store the option name. This is a change in behavior as now git checkout --conflict=bad --conflict=diff3 will error out when parsing "--conflict=bad" whereas before this change it would succeed because it would only try to parse the value of the last "--conflict" option given on the command line. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-14merge options: add a conflict style memberPhillip Wood5-2/+12
Add a conflict_style member to `struct merge_options` and `struct ll_merge_options` to allow callers to override the default conflict style. This will be used in the next commit. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-14merge-ll: introduce LL_MERGE_OPTIONS_INITPhillip Wood5-5/+6
Introduce a macro to initialize `struct ll_merge_options` in preparation for the next commit that will add a new member that needs to be initialized to a non-zero value. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-14xdiff-interface: refactor parsing of merge.conflictstylePhillip Wood2-11/+19
Factor out the code that parses of conflict style name so it can be reused in a later commit that wants to parse the name given on the command line. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-14t0006: add more tests with a negative TZ offsetBeat Bolli1-0/+8
This test doesn't systematically check a negative timezone offset. Add a test for each format that outputs the offset to improve our test coverage. Signed-off-by: Beat Bolli <dev+git@drbeat.li>
2024-03-13date: make "iso-strict" conforming for the UTC timezoneBeat Bolli2-5/+10
ISO 8601-1:2020-12 specifies that a zero timezone offset must be denoted with a "Z" suffix instead of the numeric "+00:00". Add the correponding special case to show_date() and a new test. Changing an established output format which might be depended on by scripts is always problematic, but here we choose to adhere more closely to the published standard. Reported-by: Michael Osipov <michael.osipov@innomotics.com> Signed-off-by: Beat Bolli <dev+git@drbeat.li> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-13status: allow --untracked=false and friendsJunio C Hamano5-12/+39
It is natural to expect that the "--untracked" option and the status.showuntrackedFiles configuration variable to take a Boolean value ("do you want me to show untracked files?"), but the current code takes nothing but "no" as "no, please do not show any". Allow the usual Boolean values to be given, and treat 'true' as "normal", and 'false' as "no". Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-13status: unify parsing of --untracked= and status.showUntrackedFilesJunio C Hamano2-18/+29
There are two code paths that take a string and parse it to enum untracked_status_type. Introduce a helper function and use it. As these two places handle an error differently, add an additional invalid value to the enum, and have the caller of the helper handle the error condition, instead of dying or emitting error message from the helper. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-13doc: status.showUntrackedFiles does not take "false"Jonas Wunderlich1-1/+1
The `status.showUntrackedFiles` config option only accepts the values "no", "normal" or "all", but not as this part of the man page suggested "false". While we are at it, camel-case the name of the variable. Signed-off-by: Jonas Wunderlich <git@03j.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12t5601: exercise clones with "includeIf.*.onbranch"Patrick Steinhardt1-0/+12
It was reported that git-clone(1) started to fail in Git v2.44 when cloning via HTTPS when the config contains an "includeIf.*.onbranch" condition: $ git clone https://example.com/repo.git Cloning into 'repo'... BUG: refs.c:2083: reference backend is unknown error: git-remote-https died of signal 6 This regression was bisected to 0fcc285c5e (refs: refactor logic to look up storage backends, 2023-12-29). This commit tightens the logic to look up ref backends such that we now die when the backend has not yet been detected by reading the gitconfig. Now on its own, this commit wouldn't have caused the failure. But in 18c9cb7524 (builtin/clone: create the refdb with the correct object format, 2023-12-12) we have also changed how git-clone(1) initializes the refdb such that it happens after the remote helper is spawned, which is required so that we can first learn about the object format used by the remote repository before initializing the refdb. Starting with this change, the remote helper will be unable to detect the repository right from the start and thus have an unconfigured ref backend. Consequently, when we try to resolve the "includeIf.*.onbranch" condition, we will now fail to look up the refdb and die. This regression has already been fixed via 199f44cb2e (builtin/clone: allow remote helpers to detect repo, 2024-02-27), where we now pre-initialize a partial refdb so that the remote helper can detect the repository right from the start. But it's clear that we're lacking test coverage of this functionality. Add a test to avoid regressing in the future. Note that this test stops short of defining the desired behaviour for the "onbranch" condition during a clone. It's not quite clear how exactly it should behave, so this is a leftover bit for the future. Reported-by: Angelo Dureghello <angelo@kernel-space.org> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12Documentation/user-manual.txt: example for generating object hashesDirk Gouders1-2/+34
Add a simple example on how object hashes can be generated manually. Further, because the document suggests to have a look at the initial commit, clarify that some details changed since that time. Signed-off-by: Dirk Gouders <dirk@gouders.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12config: allow multi-byte core.commentCharJeff King5-8/+27
Now that all of the code handles multi-byte comment characters, it's safe to allow users to set them. There is one special case I kept: we still will not allow an empty string for the commentChar. While it might make sense in some contexts (e.g., output where you don't want any comment prefix), there are plenty where it will behave badly (e.g., all of our starts_with() checks will indicate that every line is a comment!). It might be reasonable to assign some meaningful semantics, but it would probably involve checking how each site behaves. In the interim let's forbid it and we can loosen things later. Likewise, the "commentChar cannot be a newline" rule is now extended to "it cannot contain a newline" (for the same reason: it can confuse our parsing loops). Since comment_line_str is used in many parts of the code, it's hard to cover all possibilities with tests. We can convert the existing double-semicolon prefix test to show that "git status" works. And we'll give it a more challenging case in t7507, where we confirm that git-commit strips out the commit template along with any --verbose text when reading the edited commit message back in. That covers the basics, though it's possible there could be issues in more exotic spots (e.g., the sequencer todo list uses its own code). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12environment: drop comment_line_char compatibility macroJeff King1-1/+0
There is no longer any code which references the single-byte comment_line_char. Let's drop it, clearing the way for true multi-byte entries in comment_line_str. It's possible there are topics in flight that have added new references to comment_line_char. But we would prefer to fail compilation (and then fix it) upon merging with this, rather than have them quietly ignore the bytes after the first. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12wt-status: drop custom comment-char stringificationJeff King1-10/+3
In wt_longstatus_print_tracking() we may conditionally show a comment prefix based on the wt_status->display_comment_prefix flag. We handle that by creating a local "comment_line_string" that is either the empty string or the comment character followed by a space. For a single-byte comment, the maximum length of this string is 2 (plus a NUL byte). But to handle multi-byte comment characters, it can be arbitrarily large. One way to handle this is to just call xstrfmt("%s ", comment_line_str), and then free it when we're done. But we can simplify things further by just conditionally switching between our prefix string and an empty string when formatting. We couldn't just do that with the previous code, because the comment character was a single byte. There's no way to have a "%c" format switch between some character and "no character at all". Whereas with "%s" you can switch between some string and the empty string. So now that we have a comment string and not a comment char, we can just use it directly when formatting. Do note that we have to also conditionally add the trailing space at the same time. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12sequencer: handle multi-byte comment characters when writing todo listJeff King1-1/+3
We already match multi-byte comment characters in parse_insn_line(), thanks to the previous commit, yielding a TODO_COMMENT entry. But in todo_list_to_strbuf(), we may call command_to_char() to convert that back into something we can output. We can't just return comment_line_char anymore, since it may require multiple bytes. Instead, we'll return "0" for this case, which is the same thing we'd return for a command which does not have a single-letter abbreviation (e.g., "revert" or "noop"). There is only a single caller of command_to_char(), and upon seeing "0" it falls back to outputting the full name via command_to_string(). So we can handle TODO_COMMENT there, returning the full string. Note that there are many other callers of command_to_string(), which will now behave differently if they pass TODO_COMMENT. But we would not expect that to happen; prior to this commit, the function just calls die() in this case. And looking at those callers, that makes sense; e.g., do_pick_commit() will only be called when servicing a pick command, and should never be called for a comment in the first place. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12find multi-byte comment chars in unterminated buffersJeff King5-5/+18
As with the previous patch, we need to swap out single-byte matching for something like starts_with() to match all bytes of a multi-byte comment character. But for cases where the buffer is not NUL-terminated (and we instead have an explicit size or end pointer), it's not safe to use starts_with(), as it might walk off the end of the buffer. Let's introduce a new starts_with_mem() that does the same thing but also accepts the length of the "haystack" str and makes sure not to walk past it. Note that in most cases the existing code did not need a length check at all, since it was written in a way that knew we had at least one byte available (and that was all we checked). So I had to read each one to find the appropriate bounds. The one exception is sequencer.c's add_commented_lines(), where we can actually get rid of the length check. Just like starts_with(), our starts_with_mem() handles an empty haystack variable by not matching (assuming a non-empty prefix). A few notes on the implementation of starts_with_mem(): - it would be equally correct to take an "end" pointer (and indeed, many of the callers have this and have to subtract to come up with the length). I think taking a ptr/size combo is a more usual interface for our codebase, though, and has the added benefit that the function signature makes it harder to mix up the three parameters. - we could obviously build starts_with() on top of this by passing strlen(str) as the length. But it's possible that starts_with() is a relatively hot code path, and it should not pay that penalty (it can generally return an answer proportional to the size of the prefix, not the whole string). - it naively feels like xstrncmpz() should be able to do the same thing, but that's not quite true. If you pass the length of the haystack buffer, then strncmp() finds that a shorter prefix string is "less than" than the haystack, even if the haystack starts with the prefix. If you pass the length of the prefix, then you risk reading past the end of the haystack if it is shorter than the prefix. So I think we really do need a new function. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12find multi-byte comment chars in NUL-terminated stringsJeff King4-4/+4
Several parts of the code need to identify lines that begin with the comment character, and do so with a simple byte equality check. As part of the transition to handling multi-byte characters, we need to match all of the bytes. For cases where we are looking in a NUL-terminated string, we can just use starts_with(), which checks all of the characters in comment_line_str. Note that we can drop the "line.len" check in wt-status.c's read_rebase_todolist(). The starts_with() function handles the case of an empty haystack buffer (it will always return false for a non-empty prefix). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12prefer comment_line_str to comment_line_char for printingJeff King8-32/+32
As part of our transition to multi-byte comment characters, we should use the string variable rather than the historical character variable. All of the sites adjusted here are just swapping out "%c" for "%s" in format strings, or strbuf_addch() for strbuf_addstr(). The type system and printf-attribute give the compiler enough information to make sure our formats and variable changes all match (especially important for cases where the format string is defined far away from its use, like prepare_to_commit() in commit.c). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12strbuf: accept a comment string for strbuf_add_commented_lines()Jeff King8-31/+21
As part of our transition to multi-byte comment characters, let's take a NUL-terminated string pointer for strbuf_add_commented_lines() rather than a single character. All of the callers have to be adjusted; most can just pass comment_line_str rather than comment_line_char. And now our "cheat" in strbuf_commented_addf() can go away, as we can take the full string from it. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12strbuf: accept a comment string for strbuf_commented_addf()Jeff King9-18/+24
As part of our transition to multi-byte comment characters, let's take a NUL-terminated string pointer for strbuf_commented_addf() rather than a single character. All of the callers have to be adjusted, but they can just pass comment_line_str rather than comment_line_char. Note that we rely on strbuf_add_commented_lines() under the hood, so we'll cheat a bit to squeeze our string into a single character (for now the two are equivalent, and we'll address this TODO in the next patch). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12strbuf: accept a comment string for strbuf_stripspace()Jeff King13-20/+20
As part of our transition to multi-byte comment characters, let's take a NUL-terminated string pointer for strbuf_stripspace(), rather than a single character. We can continue to support its feature of ignoring comments by accepting a NULL pointer (as opposed to the current behavior of a NUL byte). All of the callers have to be adjusted, but they can all just pass comment_line_str (or NULL). Inside the function we detect comments by comparing the first byte of a line to the comment character. We'll adjust that to use starts_with(), which will match multiple bytes (though for now, of course, we still only allow a single byte, so it's academic). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12environment: store comment_line_char as a stringJeff King4-5/+6
We'd like to eventually support multi-byte comment prefixes, but the comment_line_char variable is referenced in many spots, making the transition difficult. Let's start by storing the character in a NUL-terminated string. That will let us switch code over incrementally to the string format, and we can easily support the existing code with a macro wrapper (since we'll continue to allow only a single-byte prefix, this will behave identically). Once all references to the "char" variable have been converted, we can drop it and enable longer strings. We'll still have to touch all of the spots that create or set the variable in this patch, but there are only a few (reading the config, and the "auto" character selector). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12strbuf: avoid shadowing global comment_line_char nameJeff King2-12/+12
Several comment-related strbuf functions take a comment_line_char parameter. There's also a global comment_line_char variable, which is closely related (most callers pass it in as this parameter). Let's avoid shadowing the global name. This makes it more obvious that we're not using the global value, and it will be especially helpful as we refactor the global in future patches (in particular, any macro trickery wouldn't work because the preprocessor doesn't respect scope). We'll use "comment_prefix". That should be descriptive enough, and as a bonus is more neutral with respect to the "char" type (since we'll eventually swap it out for a string). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12commit: refactor base-case of adjust_comment_line_char()Jeff King1-2/+3
When core.commentChar is set to "auto", we check a set of candidate characters against the proposed buffer to see which if any can be used without ambiguity. But before we do that, we optimize for the common case that the default "#" is fine by just seeing if it is present in the buffer at all. The way we do this is a bit subtle, though: we assign the candidate character to comment_line_char preemptively, then check if it works, and return if it does. The subtle part is that sometimes setting comment_line_char is important (after we return, the important outcome is the fact that we have set the variable) and sometimes it is useless (if our optimization fails, we go on to do the more careful checks and eventually assign something else instead). To make it more clear what is happening (and to make further refactoring of comment_line_char easier), let's check our candidate character directly, and then assign as part of returning if it worked out. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12strbuf: avoid static variables in strbuf_add_commented_lines()Jeff King1-3/+3
In strbuf_add_commented_lines(), we have to convert the single-byte comment_line_char into a string to pass to add_lines(). We cache the created string using a static-local variable. But this makes the function non-reentrant, and it's doubtful that this provides any real performance benefit given that we know the string always contains a single character. So let's just create it from scratch each time, and to give the compiler the maximal opportunity to make it fast we'll ditch the over-complicated xsnprintf() and just assign directly into the array. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12strbuf: simplify comment-handling in add_lines() helperJeff King1-14/+10
In strbuf_add_commented_lines(), we prepare two strings with potential prefixes: one with just the comment char, and one with an additional space. In the add_lines() helper, we use the one without the extra space for blank lines or lines starting with a tab. While passing in two separate prefixes to the helper is very flexible, it's more flexibility than we actually use (or are likely to use, since the rules inside add_lines() only make sense if "prefix2" is a variant of "prefix1" without the extra space). And setting up the two strings makes refactoring in strbuf_add_commented_lines() awkward. Instead, let's pass in a single string, and just let add_lines() add the extra space to the result as appropriate. We do still need to pass in a flag to trigger this behavior. The helper is shared by strbuf_add_lines(), which passes in a NULL "prefix2" to inhibit this extra handling. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12config: forbid newline as core.commentCharJeff King2-0/+7
Since we usually look for a comment char while parsing line-oriented files, setting core.commentChar to a single newline can confuse our code quite a bit. For example, using it with "git commit" causes us to fail to recognize any of the template as comments, including it in the config message. Which kind of makes sense, since the template content is on its own line (so no line can "start" with a newline). In other spots I would not be surprised if you can create more mischief (e.g., violating loop assumptions) but I didn't dig into it. Since comment characters are a local preference, to some degree this is a case of "if it hurts, don't do it". But given that this would be a silly and pointless thing to do, and that it makes it harder to reason about code parsing comment lines, let's just forbid it. There are other cases that are perhaps questionable (e.g., setting the comment char to a single space), but they seem to behave reasonably (at least a simple "git commit" will correctly identify and strip the template lines). So I haven't worried about going on a hunt for every stupid thing a user might do to themselves, and just focused on the most confusing case. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-11The sixth batchJunio C Hamano1-0/+4
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-11Merge branch 'sj/t9117-path-is-file'Junio C Hamano1-8/+8
GSoC practice to replace "test -f" with "test_path_is_file". * sj/t9117-path-is-file: t9117: prefer test_path_* helper functions
2024-03-11Merge branch 'kh/doc-dashed-commands-have-not-worked-for-a-long-time'Junio C Hamano1-3/+0
Doc update. * kh/doc-dashed-commands-have-not-worked-for-a-long-time: gitcli: drop mention of “non-dashed form”
2024-03-11Merge branch 'rs/t-ctype-simplify'Junio C Hamano1-54/+27
Code simplification to one unit-test program. * rs/t-ctype-simplify: t-ctype: avoid duplicating class names t-ctype: align output of i t-ctype: simplify EOF check t-ctype: allow NUL anywhere in the specification string
2024-03-11Merge branch 'es/config-doc-sort-sections'Junio C Hamano1-14/+14
Doc updates. * es/config-doc-sort-sections: docs: sort configuration variable groupings alphabetically
2024-03-11Merge branch 'js/merge-base-with-missing-commit'Junio C Hamano29-188/+460
Make sure failure return from merge_bases_many() is properly caught. * js/merge-base-with-missing-commit: merge-ort/merge-recursive: do report errors in `merge_submodule()` merge-recursive: prepare for `merge_submodule()` to report errors commit-reach(repo_get_merge_bases_many_dirty): pass on errors commit-reach(repo_get_merge_bases_many): pass on "missing commits" errors commit-reach(get_octopus_merge_bases): pass on "missing commits" errors commit-reach(repo_get_merge_bases): pass on "missing commits" errors commit-reach(get_merge_bases_many_0): pass on "missing commits" errors commit-reach(merge_bases_many): pass on "missing commits" errors commit-reach(paint_down_to_common): start reporting errors commit-reach(paint_down_to_common): prepare for handling shallow commits commit-reach(repo_in_merge_bases_many): report missing commits commit-reach(repo_in_merge_bases_many): optionally expect missing commits commit-reach(paint_down_to_common): plug two memory leaks