| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
The original reasoning for not removing section headers upon removal of
the last entry went like this: the user could have added comments about
the section, or about the entries therein, and if there were other
comments there, we would not know whether we should remove them.
In particular, a concocted example was presented that looked like this
(and was added to t1300):
# some generic comment on the configuration file itself
# a comment specific to this "section" section.
[section]
# some intervening lines
# that should also be dropped
key = value
# please be careful when you update the above variable
The ideal thing for `git config --unset section.key` in this case would
be to leave only the first line behind, because all the other comments
are now obsolete.
However, this is unfeasible, short of adding a complete Natural Language
Processing module to Git, which seems not only a lot of work, but a
totally unreasonable feature (for little benefit to most users).
Now, the real kicker about this problem is: most users do not edit their
config files at all! In their use case, the config looks like this
instead:
[section]
key = value
... and it is totally obvious what should happen if the entry is
removed: the entire section should vanish.
Let's generalize this observation to this conservative strategy: if we
are removing the last entry from a section, and there are no comments
inside that section nor surrounding it, then remove the entire section.
Otherwise behave as before: leave the now-empty section (including those
comments, even ones about the now-deleted entry).
We have to be extra careful to handle the case where more than one entry
is removed: any subset of them might be the last entries of their
respective sections (and if there are no comments in or around that
section, the section should be removed, too).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
In the recent commit with the title "config: introduce an optional event
stream while parsing", we introduced an optional callback to keep track
of the config parser's events "comment", "white-space", "section header"
and "entry".
One motivation for this feature was to make use of it in the code that
edits the config. And this commit makes it so.
Note: this patch changes the meaning of the `seen` array that records
whether we saw the config entry that is to be edited: previously, it
contained the end offset of the found entry. Now, we introduce a new
array `parsed` that keeps a record of *all* config parser events (with
begin/end offsets), and the items in the `seen` array now point into the
`parsed` array.
There are two reasons why we do it this way:
1. To keep the implementation simple, the config parser's event stream
reports the event only after the config callback was called, so we
would not receive the begin offset otherwise.
2. In the following patches, we will re-use the `parsed` array to fix two
long-standing bugs related to empty sections.
Note that this also makes the code more robust with respect to finding the
begin offset of the part(s) of the config file to be edited, as we no
longer back-track to find the beginning of the line.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
While a neat theoretical construct, state machines are hard to read. In
this instance, it does not even make a whole lot of sense because we are
more interested in flags, anyway: has the section been seen? Has the key
been seen? Does the current section match the key we are looking for?
Besides, the state `SECTION_SEEN` was named in a misleading way: it did
not indicate that we saw the section matching the key we are looking
for, but it instead indicated that we are *currently* in that section.
Let's just replace the state machine logic by clear and obvious flags.
This will also make it easier to review the upcoming patches to use the
newly-introduced `event_fn` callback of the config parser.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The `seen` field is the actual length of the `offset` array, and the
`offset_alloc` field records what was allocated (to avoid resizing
wherever `seen` has to be incremented).
Elsewhere, we use the convention `name` for the array, where `name` is
descriptive enough to guess its purpose, `name_nr` for the actual length
and `name_alloc` to record the maximum length without needing to resize.
Let's make the names of the fields in question consistent with that
convention.
This will also help with the next steps where we will let the
git_config_set() machinery use the config event stream that we just
introduced.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
It is much easier to reason about, when the config code to set/unset
variables or to remove/rename sections does not rely on a global (or
file-local) variable.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
This extends our config parser so that it can optionally produce an event
stream via callback function, where it reports e.g. when a comment was
parsed, or a section header, etc.
This parser will be used subsequently to handle the scenarios better where
removing config entries would make sections empty, or where a new entry
could be added to an already-existing, empty section.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
We already have a test demonstrating that removing the last entry from a
config section fails to remove the section header of the now-empty
section.
The same can happen, of course, if we remove the last entries in one fell
swoop. This is *also* a bug, and should be fixed at the same time.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
During the review of the first iteration of the patch series to remove
sections that become empty upon --unset or --unset-all, Jeff King
identified a couple of problematic cases with the backtracking approach
that was still used then to "look backwards for the section header":
https://public-inbox.org/git/20180329213229.GG2939@sigill.intra.peff.net/
This patch adds a couple of concocted examples designed to fool a
backtracking parser.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
In https://public-inbox.org/git/7vvc8alzat.fsf@alter.siamese.dyndns.org/
a reasonable patch was made quite a bit less so by changing a test case
demonstrating a bug to a test case that demonstrates that we ask for too
much: the test case 'unsetting the last key in a section removes header'
now expects a future bug fix to be able to determine whether a free-form
comment above a section header refers to said section or not.
Rather than shooting for the stars (and not even getting off the
ground), let's start shooting for something obtainable and be reasonably
confident that we *can* get it.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The test case 'unset with cont. lines' relied on a bug that is about to
be fixed: it tests *explicitly* that removing the last entry from a
config section leaves an *empty* section behind.
Let's fix this test case not to rely on that behavior, simply by
preventing the section from becoming empty.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When replacing multiple config entries at once, we did not re-set the
flag that indicates whether we need to insert a new-line before the new
entry. As a consequence, an extra new-line was inserted under certain
circumstances.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Currently, we are slightly overzealous When removing an entry from a
config file of this form:
[abc]a
[xyz]
key = value
When calling `git config --unset abc.a` on this file, it leaves this
(invalid) config behind:
[
[xyz]
key = value
The reason is that we try to search for the beginning of the line (or
for the end of the preceding section header on the same line) that
defines abc.a, but as an optimization, we subtract 2 from the offset
pointing just after the definition before we call
find_beginning_of_line(). That function, however, *also* performs that
optimization and promptly fails to find the section header correctly.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Doc markup fix.
* ms/non-ascii-ticks:
Documentation/gitsubmodules.txt: avoid non-ASCII apostrophes
|
|
Code clean-up.
* jk/cached-commit-buffer:
revision: drop --show-all option
commit: drop uses of get_cached_commit_buffer()
|
|
Code clean-up.
* sm/mv-dry-run-update:
mv: remove unneeded 'if (!show_only)'
t7001: add test case for --dry-run
|
|
Hotfix for a recent topic.
* tg/worktree-create-tracking:
git-worktree.txt: fix indentation of example and text of 'add' command
git-worktree.txt: fix missing ")" typo
|
|
Test update.
* gs/test-unset-xdg-cache-home:
test-lib.sh: unset XDG_CACHE_HOME
|
|
Docfix.
* sb/status-doc-fix:
Documentation/git-status: clarify status table for porcelain mode
|
|
Typofix.
* rd/typofix:
Correct mispellings of ".gitmodule" to ".gitmodules"
t/: correct obvious typo "detahced"
|
|
Doc update for a recently added feature.
* bp/fsmonitor:
fsmonitor: update documentation to remove reference to invalid config settings
|
|
Docfix.
* bc/doc-interpret-trailers-grammofix:
docs/interpret-trailers: fix agreement error
|
|
Devdoc update.
* sg/doc-test-must-fail-args:
t: document 'test_must_fail ok=<signal-name>'
|
|
Devtool update.
* rj/sparse-updates:
Makefile: suppress a sparse warning for pack-revindex.c
config.mak.uname: remove SPARSE_FLAGS setting for cygwin
|
|
Test updates.
* jk/gettext-poison:
git-sh-i18n: check GETTEXT_POISON before USE_GETTEXT_SCHEME
t0205: drop redundant test
|
|
Doc update.
* nd/ignore-glob-doc-update:
gitignore.txt: elaborate shell glob syntax
|
|
* rs/cocci-strbuf-addf-to-addstr:
cocci: simplify check for trivial format strings
|
|
Error message fix.
* jc/worktree-add-short-help:
worktree: say that "add" takes an arbitrary commit in short-help
|
|
Doc update.
* tz/doc-show-defaults-to-head:
doc: mention 'git show' defaults to HEAD
|
|
Code clean-up.
* nd/shared-index-fix:
read-cache: don't write index twice if we can't write shared index
read-cache.c: move tempfile creation/cleanup out of write_shared_index
read-cache.c: change type of "temp" in write_shared_index()
|
|
Corner case bugfix.
* jc/mailinfo-cleanup-fix:
mailinfo: avoid segfault when can't open files
|
|
Code clean-up.
* rb/hashmap-h-compilation-fix:
hashmap.h: remove unused variable
|
|
Code clean-up.
* rs/describe-unique-abbrev:
describe: use strbuf_add_unique_abbrev() for adding short hashes
|
|
Doc updates.
* ks/submodule-doc-updates:
Doc/git-submodule: improve readability and grammar of a sentence
Doc/gitsubmodules: make some changes to improve readability and syntax
|
|
Test clean-up.
* cl/t9001-cleanup:
t9001: use existing helper in send-email test
|
|
Code clean-up.
* bw/oidmap-autoinit:
oidmap: ensure map is initialized
|
|
Test fixes.
* sg/test-i18ngrep:
t: make 'test_i18ngrep' more informative on failure
t: validate 'test_i18ngrep's parameters
t: move 'test_i18ncmp' and 'test_i18ngrep' to 'test-lib-functions.sh'
t5536: let 'test_i18ngrep' read the file without redirection
t5510: consolidate 'grep' and 'test_i18ngrep' patterns
t4001: don't run 'git status' upstream of a pipe
t6022: don't run 'git merge' upstream of a pipe
t5812: add 'test_i18ngrep's missing filename parameter
t5541: add 'test_i18ngrep's missing filename parameter
|
|
Plug recently introduced leaks in fsck.
* jt/fsck-code-cleanup:
fsck: fix leak when traversing trees
|
|
Workaround for segfault with more recent versions of SVN.
* ew/svn-branch-segfault-fix:
git-svn: control destruction order to avoid segfault
|
|
Completion of "git merge -s<strategy>" (in contrib/) did not work
well in non-C locale.
* nd/list-merge-strategy:
completion: fix completing merge strategies on non-C locales
|
|
Assorted fixes to "git daemon".
* jk/daemon-fixes:
daemon: fix length computation in newline stripping
t/lib-git-daemon: add network-protocol helpers
daemon: handle NULs in extended attribute string
daemon: fix off-by-one in logging extended attributes
t/lib-git-daemon: record daemon log
t5570: use ls-remote instead of clone for interp tests
|
|
The split-index mode had a few corner case bugs fixed.
* tg/split-index-fixes:
travis: run tests with GIT_TEST_SPLIT_INDEX
split-index: don't write cache tree with null oid entries
read-cache: fix reading the shared index for other repos
|
|
Crash fix for a corner case where an error codepath tried to unlock
what it did not acquire lock on.
* mr/packed-ref-store-fix:
files_initial_transaction_commit(): only unlock if locked
|
|
The http tracing code, often used to debug connection issues,
learned to redact potentially sensitive information from its output
so that it can be more safely sharable.
* jt/http-redact-cookies:
http: support omitting data from traces
http: support cookie redaction when tracing
|
|
Avoid showing a warning message in the middle of a line of "git
diff" output.
* nd/diff-flush-before-warning:
diff.c: flush stdout before printing rename warnings
|
|
Build the executable in 'script' phase in Travis CI integration, to
follow the established practice, rather than during 'before_script'
phase. This allows the CI categorize the failures better ('failed'
is project's fault, 'errored' is build environment's).
* sg/travis-build-during-script-phase:
travis-ci: build Git during the 'script' phase
|
|
Fix for a commented-out code to adjust it to a rather old API change.
* ys/bisect-object-id-missing-conversion-fix:
bisect: debug: convert struct object to object_id
|
|
When resetting the working tree files recursively, the working tree
of submodules are now also reset to match.
* sb/submodule-update-reset-fix:
submodule: submodule_move_head omits old argument in forced case
unpack-trees: oneway_merge to update submodules
t/lib-submodule-update.sh: fix test ignoring ignored files in submodules
t/lib-submodule-update.sh: clarify test
|
Johannes Schindelin
Junio C Hamano