The error message shown when a flag is found when expecting a
filename wasn't clear as it didn't communicate what was wrong
using the 'suitable' words in *all* cases.
$ git ls-files
README.md
test-file
Correct case,
$ git rev-parse README.md --flags
README.md
--flags
fatal: bad flag '--flags' used after filename
Incorrect case,
$ git grep "some random regex" -n
fatal: bad flag '-n' used after filename
The above case is incorrect as "some random regex" isn't a filename
in this case.
Change the error message to be general and communicative. This results
in the following output,
$ git rev-parse README.md --flags
README.md
--flags
fatal: option '--flags' must come before non-option arguments
$ git grep "some random regex" -n
fatal: option '-n' must come before non-option arguments
Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The error messages shown when the branch command is misused
by supplying it wrong number of parameters wasn't meaningful.
That's because it used the the phrase "too many branches"
assuming all parameters to be "valid" branch names. It's not
always the case as exemplified below,
$ git branch
foo
* master
$ git branch -m foo foo old
fatal: too many branches for a rename operation
Change the messages to be more general thus making no assumptions
about the "parameters".
Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* jk/ui-color-always-to-auto-maint:
color: make "always" the same as "auto" in config
provide --color option for all ref-filter users
t3205: use --color instead of color.branch=always
t3203: drop "always" color test
t6006: drop "always" color config tests
t7502: use diff.noprefix for --verbose test
t7508: use test_terminal for color output
t3701: use test-terminal to collect color output
t4015: prefer --color to -c color.diff=always
test-terminal: set TERM=vt100
This test wants to confirm that "clean -i" shows color
output. Using test_terminal gives us a more realistic
environment than "color.ui=always", and prepares us for the
behavior of "always" changing in a future patch.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The tests for --color-moved write their output to a file,
but doing so suppresses color output under "auto". Right now
this is solved by running the whole script under
"color.diff=always". In preparation for the behavior of
"always" changing, let's explicitly enable color.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It can be handy to use `--color=always` (or it's synonym
`--color`) on the command-line to convince a command to
produce color even if it's stdout isn't going to the
terminal or a pager.
What's less clear is whether it makes sense to set config
variables like color.ui to `always`. For a one-shot like:
git -c color.ui=always ...
it's potentially useful (especially if the command doesn't
directly support the `--color` option). But setting `always`
in your on-disk config is much muddier, as you may be
surprised when piped commands generate colors (and send them
to whatever is consuming the pipe downstream).
Some people have done this anyway, because:
1. The documentation for color.ui makes it sound like
using `always` is a good idea, when you almost
certainly want `auto`.
2. Traditionally not every command (and especially not
plumbing) respected color.ui in the first place. So
the confusion came up less frequently than it might
have.
The situation changed in 136c8c8b8f (color: check color.ui
in git_default_config(), 2017-07-13), which negated point
(2): now scripts using only plumbing commands (like
add-interactive) are broken by this setting.
That commit was fixing real issues (e.g., by making
`color.ui=never` work, since `auto` is the default), so we
don't want to just revert it. We could turn `always` into a
noop in plumbing commands, but that creates a hard-to-explain
inconsistency between the plumbing and other commands.
Instead, let's just turn `always` into `auto` for all config.
This does break the "one-shot" config shown above, but again,
we're probably better to have simple and consistent rules than
to try to special-case command-line config.
There is one place where `always` should retain its meaning:
on the command line, `--color=always` should continue to be
the same as `--color`, overriding any isatty checks. Since the
command-line parser also depends on git_config_colorbool(), we
can use the existence of the "var" string to deterine whether
we are serving the command-line or the config.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When ref-filter learned about want_color() in 11b087adfd
(ref-filter: consult want_color() before emitting colors,
2017-07-13), it became useful to be able to turn colors off
and on for specific commands. For git-branch, you can do so
with --color/--no-color.
But for git-for-each-ref and git-tag, the other users of
ref-filter, you have no option except to tweak the
"color.ui" config setting. Let's give both of these commands
the usual color command-line options.
This is a bit more obvious as a method for overriding the
config. And it also prepares us for the behavior of "always"
changing (so that we are still left with a way of forcing
color when our output goes to a non-terminal).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
To test the color output, we must convince "git branch" to
write colors to a non-terminal. We do that now by setting
the color config to "always". In preparation for the
behavior of "always" changing, let's switch to using the
"--color" command-line option, which is more direct.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In preparation for the behavior of "always" changing to
match "auto", we can simply drop this test. We already check
other forms (like "--color") independently.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We test the %C() format placeholders with a variety of
color-inducing options, including "--color" and
"-c color.ui=always". In preparation for the behavior of
"always" changing, we need to do something with those
"always" tests.
We can drop ones that expect "always" to turn on color even
to a file, as that will become a synonym for "auto", which
is already tested.
For the "--no-color" test, we need to make sure that color
would otherwise be shown. To do this, we can use
test_terminal, which enables colors in the default setup.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
To check that "status -v" respects diff config, we set
"color.diff" and look at the output of "status". We could
equally well use any diff config. Since color output depends
on a lot of other factors (like whether stdout is a tty, and
how we interpret "always"), let's use a more mundane option.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This script tests the output of status with various formats
when color is enabled. It uses the "always" setting so that
the output is valid even though we capture it in a file.
Using test_terminal gives us a more realistic environment,
and prepares us for the behavior of "always" changing.
Arguably we are testing less than before, since "auto" is
already the default, and we can no longer tell if the config
is actually doing anything.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When testing whether "add -p" can generate colors, we set
color.ui to "always". This isn't a very good test, as in the
real-world a user typically has "auto" coupled with stdout
going to a terminal (and it's plausible that this could mask
a real bug in add--interactive if we depend on plumbing's
isatty check).
Let's switch to test_terminal, which gives us a more
realistic environment. This also prepare us for future
changes to the "always" color option.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
t4015 contains many color-related tests which need to
override the "is stdout a tty" check. They do so by setting
the color.diff config, but we can accomplish the same with
the --color option. Besides being shorter to type, switching
will prepare us for upcoming changes to "always" when see it
in config.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The point of the test-terminal script is to simulate in the
test scripts an environment where output is going to a real
terminal.
But since test-lib.sh also sets TERM=dumb, the simulation
isn't very realistic. The color code will skip auto-coloring
for TERM=dumb, leading to us liberally sprinkling
test_terminal env TERM=vt100 git ...
through the test suite to convince the tests to actually
generate colors. Let's set TERM for programs run under
test_terminal, which is one less thing for test-writers to
remember.
In most cases the callers can be simplified, but note there
is one interesting case in t4202. It uses test_terminal to
check the auto-enabling of --decorate, but the expected
output _doesn't_ contain colors (because TERM=dumb
suppresses them). Using TERM=vt100 is closer to what the
real world looks like; adjust the expected output to match.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Operations that do not touch (majority of) packed refs have been
optimized by making accesses to packed-refs file lazy; we no longer
pre-parse everything, and an access to a single ref in the
packed-refs does not touch majority of irrelevant refs, either.
* mh/mmap-packed-refs: (21 commits)
packed-backend.c: rename a bunch of things and update comments
mmapped_ref_iterator: inline into `packed_ref_iterator`
ref_cache: remove support for storing peeled values
packed_ref_store: get rid of the `ref_cache` entirely
ref_store: implement `refs_peel_ref()` generically
packed_read_raw_ref(): read the reference from the mmapped buffer
packed_ref_iterator_begin(): iterate using `mmapped_ref_iterator`
read_packed_refs(): ensure that references are ordered when read
packed_ref_cache: keep the `packed-refs` file mmapped if possible
packed-backend.c: reorder some definitions
mmapped_ref_iterator_advance(): no peeled value for broken refs
mmapped_ref_iterator: add iterator over a packed-refs file
packed_ref_cache: remember the file-wide peeling state
read_packed_refs(): read references with minimal copying
read_packed_refs(): make parsing of the header line more robust
read_packed_refs(): only check for a header at the top of the file
read_packed_refs(): use mmap to read the `packed-refs` file
die_unterminated_line(), die_invalid_line(): new functions
packed_ref_cache: add a backlink to the associated `packed_ref_store`
prefix_ref_iterator: break when we leave the prefix
...
The output from "git diff --summary" was broken in a recent topic
that has been merged to 'master' and lost a LF after reporting of
mode change. This has been fixed.
* sb/diff-color-move:
diff: correct newline in summary for renamed files
Code clean-up.
* jk/validate-headref-fix:
validate_headref: use get_oid_hex for detached HEADs
validate_headref: use skip_prefix for symref parsing
validate_headref: NUL-terminate HEAD buffer
Code clean-up to prevent future mistakes by copying and pasting
code that checks the result of read_in_full() function.
* jk/read-in-full:
worktree: check the result of read_in_full()
worktree: use xsize_t to access file size
distinguish error versus short read from read_in_full()
avoid looking at errno for short read_in_full() returns
prefer "!=" when checking read_in_full() result
notes-merge: drop dead zero-write code
files-backend: prefer "0" for write_in_full() error check
Some commands (most notably "git status") makes an opportunistic
update when performing a read-only operation to help optimize later
operations in the same repository. The new "--no-optional-locks"
option can be passed to Git to disable them.
* jk/no-optional-locks:
git: add --no-optional-locks option
Add comment to a few functions that use a short-lived buffer the
caller can peek and copy out of.
* hn/path-ownership-comment:
read_gitfile_gently: clarify return value ownership.
real_path: clarify return value ownership
"git branch" learned "-c/-C" to create a new branch by copying an
existing one.
* sd/branch-copy:
branch: fix "copy" to never touch HEAD
branch: add a --copy (-c) option to go with --move (-m)
branch: add test for -m renaming multiple config sections
config: create a function to format section headers
Recent versions of "git rev-parse --parseopt" did not parse the
option specification that does not have the optional flags (*=?!)
correctly, which has been corrected.
* bc/rev-parse-parseopt-fix:
parse-options: only insert newline in help text if needed
parse-options: write blank line to correct output stream
t0040,t1502: Demonstrate parse_options bugs
git-rebase: don't ignore unexpected command line arguments
rev-parse parseopt: interpret any whitespace as start of help text
rev-parse parseopt: do not search help text for flag chars
t1502: demonstrate rev-parse --parseopt option mis-parsing
The final batch to "git rebase -i" updates to move more code from
the shell script to C.
* js/rebase-i-final:
rebase -i: rearrange fixup/squash lines using the rebase--helper
t3415: test fixup with wrapped oneline
rebase -i: skip unnecessary picks using the rebase--helper
rebase -i: check for missing commits in the rebase--helper
t3404: relax rebase.missingCommitsCheck tests
rebase -i: also expand/collapse the SHA-1s via the rebase--helper
rebase -i: do not invent onelines when expanding/collapsing SHA-1s
rebase -i: remove useless indentation
rebase -i: generate the script via rebase--helper
t3415: verify that an empty instructionFormat is handled as before
Of the many ways to spell the three-letter word, the variant "Git"
should be used when referring to a repository in a description; or, in
general, when it is used as a proper noun.
We thus change the pull-request template message so that it reads
"...in the Git repository at:"
Besides, this brings us in line with the documentation, see
Documentation/howto/using-signed-tag-in-pull-request.txt
Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use the macro ALLOC_ARRAY to allocate an array. This is shorter and
easier, as it automatically infers the size of elements.
Patch generated with Coccinelle and contrib/coccinelle/array.cocci.
Signeg-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The %(contents) atom takes a contents "field" as its argument. Since
"trailers" is one of those fields, extend contents_atom_parser to parse
"trailers"'s arguments when used through "%(contents)", like:
%(contents:trailers:unfold,only)
A caveat: trailers_atom_parser expects NULL when no arguments are given
(see: `parse_ref_filter_atom`). This is because string_list_split (given
a maxsplit of -1) returns a 1-ary string_list* containing the given
string if the delimiter could not be found using `strchr`.
To simulate this behavior without teaching trailers_atom_parser to
accept strings with length zero, conditionally pass NULL to
trailers_atom_parser if the arguments portion of the argument to
%(contents) is empty.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fill trailer_opts with "unfold" and "only" to match the sub-arguments
given to the "%(trailers)" atom. Then, let's use the filled trailer_opts
instance with 'format_trailers_from_commit' in order to format trailers
in the desired manner.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We currently have one test for %(trailers) in `git-for-each-ref(1)`,
through "%(contents:trailers)". In preparation for more, let's add a few
things:
- Move the commit creation step to its own test so that it can be
re-used.
- Add a non-trailer to the commit's trailers to test that non-trailers
aren't shown using "%(trailers:only)".
- Add a multi-line trailer to ensure that trailers are unfolded
correctly using "%(trailers:unfold)".
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"'<string>'"-style quoting is not appropriate when quoting literal
strings in the "Documentation/" subtree.
In preparation for adding additional information to this section of
git-for-each-ref(1)'s documentation, update them to use "`<literal>`"
instead.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The documentation makes reference to 'contents:trailers' as an example
to dig the trailers out of a commit. 'trailers' is an unmentioned
alternative, which is treated as an alias of 'contents:trailers'.
Since 'trailers' is easier to type, prefer that as the designated way to
dig out trailers information.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Tests in t4205 test the following:
git log --format='%(trailers:unfold)' ...
By ensuring the multi-line trailers are unfolded back onto the same
line. t4205 only includes tests for 2-line trailers, but `unfold()` will
fail for folded trailers on 3 or more lines.
In preparation for adding subsequent tests in t6300 that test similar
behavior in `git-for-each-ref(1)`, let's harden t4205 (and make it
consistent with the changes in t6300) by ensuring that 3 or more
line folded trailers are unfolded correctly.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Having a .clang-format file in a project can be understood in a way that
code has to be in the style defined by the .clang-format file, i.e., you
just have to run clang-format over all code and you are set.
This unfortunately is not yet the case in the Git project, as the
format file is still work in progress. Explain it with a comment in
the beginning of the file.
Additionally, the working clang-format version is mentioned because the
config directives change from time to time (in a compatibility-breaking way).
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use the macro FREE_AND_NULL to release allocated objects and clear their
pointers. This is shorter and documents the intent better by combining
the two related operations into one.
Patch generated with Coccinelle and contrib/coccinelle/free.cocci.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
lookup_blob() etc. can return NULL if the referenced object isn't of the
expected type. In theory it's wrong to reference the object member in
that case. In practice it's OK because it's located at offset 0 for all
types, so the pointer arithmetic (NULL + 0) is optimized out by the
compiler. The issue is reported by Clang's AddressSanitizer, though.
Avoid the ASan error by casting the results of the lookup functions to
struct object pointers. That works fine with NULL pointers as well. We
already rely on the object member being first in all object types in
other places in the code.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
strbuf_addf() can be used to add a specific number of space characters
by using the format "%*s" with an empty string and specifying the
desired width. Use strbuf_addchars() instead as it's shorter, makes the
intent clearer and is a bit more efficient.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use strbuf_addstr() instead of strbuf_addf() for adding strings. That's
simpler and makes the intent clearer.
Patch generated by Coccinelle and contrib/coccinelle/strbuf.cocci;
adjusted indentation in refs/packed-backend.c manually.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Avoid a string copy to a static buffer by using strbuf_add_real_path()
instead of combining strbuf_addstr() and real_path().
Patch generated by Coccinelle and contrib/coccinelle/strbuf.cocci.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add some UNLEAKs where we are about to return from `cmd_*`. UNLEAK the
variables in the same order as we've declared them. While addressing
`msg` in builtin/tag.c, convert the existing `strbuf_release()` calls as
well.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Transformations that hide multiplications can end up with an pair of
parentheses that is no longer needed. E.g. with a rule like this:
@@
expression E;
@@
- E * 2
+ double(E)
... we might get a patch like this:
- x = (a + b) * 2;
+ x = double((a + b));
Add a pair of parentheses to the preimage side of such rules.
Coccinelle will generate patches that remove them if they are present,
and it will still match expressions that lack them.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In preparation for adding consistent "%(trailers)" atom options to
`git-for-each-ref(1)`'s "--format" argument, change "%(trailers)" in
pretty.c to separate sub-arguments with a ",", instead of a ":".
Multiple sub-arguments are given either as "%(trailers:unfold,only)" or
"%(trailers:only,unfold)".
This change disambiguates between "top-level" arguments, and arguments
given to the trailers atom itself. It is consistent with the behavior of
"%(upstream)" and "%(push)" atoms.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This allows us to get rid of several write-only variables.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
