| Age | Commit message (Collapse) | Author |
|---|
|
When parsing colors, a failed parse leads to an error message due to the
result returning error(). To allow for quiet parsing, create
color_parse_quietly(). This is in contrast to an ..._gently() version
because the original does not die(), so both options are technically
'gentle'.
To accomplish this, convert the implementation of color_parse_mem() into
a static color_parse_mem_1() helper that adds a 'quiet' parameter. The
color_parse_quietly() method can then use this. Since it is a near
equivalent to color_parse(), move that method down in the file so they
can be nearby while also appearing after color_parse_mem_1().
Signed-off-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The point of want_color() is to take in a git_colorbool enum value and
collapse it down to a single true/false boolean, letting UNKNOWN fall
back to the color.ui default and checking isatty() for AUTO.
Let's make that more clear in the type system by returning a bool rather
than an integer.
This sadly still does not help us much with compiler warnings for using
the two types interchangeably. But it helps make the intent more clear
to a human reader.
We still retain the idempotency of want_color(), because in C a bool
true/false converts to 1/0 when converted to an integer, which
corresponds to GIT_COLOR_ALWAYS and GIT_COLOR_NEVER. So you can store
the bool in a git_colorbool and get the right result (something a few
pieces of code still do, but which we'll clean up in further patches).
Note that we rely on this same bool/int conversion for
check_auto_color(). We cache its results in a tristate int with "-1" as
"not yet set", but we can assign to it (and return it) with implicit
conversions to/from bool.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
We traditionally used "int" to store and pass around the values defined
by "enum git_colorbool" (which were originally just #define macros).
Using an int doesn't produce incorrect results, but using the actual
enum makes the intent of the code more clear.
It would be nice if the compiler could catch cases where we used the
enum and an int interchangeably, since it's very easy to accidentally
check the boolean true/false of a colorbool like:
if (branch_use_color)
This is wrong because GIT_COLOR_UNKNOWN and GIT_COLOR_AUTO evaluate to
true in C, even though we may ultimately decide not to use color. But C
is pretty happy to convert between ints and enums (even with various
-Wenum-* warnings). So this sadly doesn't protect us from such mistakes,
but it hopefully does make the code easier to read.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The git_config_colorbool() function returns an integer which is always
one of the GIT_COLOR_* constants UNKNOWN, NEVER, ALWAYS, or AUTO. We
define these constants with macros, but let's switch to using an enum.
Even though the compiler does not strictly enforce enum/int conversions,
this should make the intent clearer to human readers. And as a bonus,
enum names are typically available to debuggers, making it more pleasant
to step through the code there.
This patch updates the return type of git_config_colorbool(), but holds
off on updating all of the callers. There's some trickiness to some of
them, and in the meantime it's perfectly fine to assign an enum into an
int.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Long ago Git's decision to show color for a subsytem was stored in a
tri-state variable: it could be true (1), false (0), or unknown (-1).
But since daa0c3d971 (color: delay auto-color decision until point of
use, 2011-08-17) we want to carry around a new state, "auto", which
bases the decision on the tty-ness of stdout (rather than collapsing
that "auto" state to a true/false immediately).
That commit introduced a set of GIT_COLOR_* defines to represent each
state: UNKNOWN, ALWAYS, NEVER, and AUTO. But it only used the AUTO
value, and left alone code using bare 0/1/-1 values. And of course since
then we've grown many new spots that use those bare values.
Let's switch all of these to use the named constants. That should make
the code a bit easier to read, as it is more obvious that we're
representing a color decision.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Mark code units that generate warnings with `-Wsign-compare`. This
allows for a structured approach to get rid of all such warnings over
time in a way that can be easily measured.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
RGB color parsing currently supports 24-bit values in the form #RRGGBB.
As in Cascading Style Sheets (CSS [1]), also allow to specify an RGB color
using only three digits with #RGB.
In this shortened form, each of the digits is – again, as in CSS –
duplicated to convert the color to 24 bits, e.g. #f1b specifies the same
color as #ff11bb.
In color.h, remove the '0x' prefix in the example to match the actual
syntax.
[1] https://developer.mozilla.org/en-US/docs/Web/CSS/hex-color
Signed-off-by: Beat Bolli <dev+git@drbeat.li>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
In order to further reduce all-in-one headers, separate out functions in
hex.h that do not operate on object hashes into its own file, hex-ll.h,
and update the include directives in the .c files that need only such
functions accordingly.
Signed-off-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
git_color_default_config() is a shorthand for calling two other config
callbacks. There are no other non-static functions that do this and it
will complicate our refactoring of config_fn_t so inline it instead.
Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Signed-off-by: Elijah Newren <newren@gmail.com>
Acked-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
cache.h and strbuf.[ch] had editor-related functions. Move these into
editor.[ch].
Signed-off-by: Elijah Newren <newren@gmail.com>
Acked-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Dozens of files made use of gettext functions, without explicitly
including gettext.h. This made it more difficult to find which files
could remove a dependence on cache.h. Make C files explicitly include
gettext.h if they are using it.
However, while compat/fsmonitor/fsm-ipc-darwin.c should also gain an
include of gettext.h, it was left out to avoid conflicting with an
in-flight topic.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
As reported in [1] the "UNUSED(var)" macro introduced in
2174b8c75de (Merge branch 'jk/unused-annotation' into next,
2022-08-24) breaks coccinelle's parsing of our sources in files where
it occurs.
Let's instead partially go with the approach suggested in [2] of
making this not take an argument. As noted in [1] "coccinelle" will
ignore such tokens in argument lists that it doesn't know about, and
it's less of a surprise to syntax highlighters.
This undoes the "help us notice when a parameter marked as unused is
actually use" part of 9b240347543 (git-compat-util: add UNUSED macro,
2022-08-19), a subsequent commit will further tweak the macro to
implement a replacement for that functionality.
1. https://lore.kernel.org/git/220825.86ilmg4mil.gmgdl@evledraar.gmail.com/
2. https://lore.kernel.org/git/220819.868rnk54ju.gmgdl@evledraar.gmail.com/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The callback passed to git_config() must conform to a particular
interface. But most callbacks don't actually look at the extra "void
*data" parameter. Let's mark the unused parameters to make
-Wunused-parameter happy.
Note there's one unusual case here in get_remote_default() where we
actually ignore the "value" parameter. That's because it's only checking
whether the option is found at all, and not parsing its value.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
"reset" was previously treated as a standalone special color name
representing `\e[m`. Now, it can apply to other color properties,
allowing exact specifications without implicit attribute inheritance.
For example, "reset green" now renders `\e[;32m`, which is interpreted
as "reset everything; then set foreground to green". This means the
background and other attributes are also reset to their defaults.
Previously, this was impossible to represent in a single color:
"reset" could be specified alone, or a color with attributes, but some
thing like clearing a background color were impossible.
There is a separate change that introduces the "default" color name to
assist with that, but even then, the above could only to be represented
by explicitly disabling each of the attributes:
green default no-bold no-dim no-italic no-ul no-blink no-reverse no-strike
Signed-off-by: Robert Estelle <robertestelle@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The name "default" can now be used in foreground or background colors,
and means to use the terminal's default color, discarding any
explicitly-set color without affecting the other attributes. On many
modern terminals, this is *not* the same as specifying "white" or
"black".
Although attributes could previously be cleared like "no-bold", there
had not been a similar mechanism available for colors, other than a full
"reset", which cannot currently be combined with other settings.
Note that this is *not* the same as the existing name "normal", which is
a no-op placeholder to permit setting the background without changing
the foreground. (i.e. what is currently called "normal" might have been
more descriptively named "inherit", "none", "pass" or similar).
Signed-off-by: Robert Estelle <robertestelle@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
This results in shorter output, and is _probably_ more portable. There
is at least one environment (GitHub Actions) which supports 16-color
mode but not 256-color mode. It's possible there are environments
which go the other way, but it seems unlikely.
Signed-off-by: Eyal Soha <shawarmakarma@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
These colors are the bright variants of the 3-bit colors. Instead of
30-37 range for the foreground and 40-47 range for the background,
they live in 90-97 and 100-107 range, respectively.
Signed-off-by: Eyal Soha <shawarmakarma@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
color_output() takes a "type" parameter, which is either '3' or '4',
and that byte is shown in front of '0'-'7' to form "30"-"37" or
"40"-"47" in ANSI output mode for fore-ground and back-ground
colors.
Clarify the purpose of the parameter by renaming it to the
"background" that is a boolean.
Also, change the .value field in the color struct from storing 0-7
for basic 8 colors to storing 30-37 for ANSI colors. This aligns
the code to show ANSI colors to the code for the 256 color scheme,
which already uses the actual value to be sent to the terminal.
Signed-off-by: Eyal Soha <shawarmakarma@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
want_color_fd() is designed to work only with standard output and
error file descriptors and stores information about each descriptor in
an array. However, it doesn't verify that the passed-in descriptor
lives within that set, which, with a buggy caller, could lead to
access or assignment outside the array bounds.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Acked-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Developer support update, by using BUG() macro instead of die() to
mark codepaths that should not happen more clearly.
* js/use-bug-macro:
BUG_exit_code: fix sparse "symbol not declared" warning
Convert remaining die*(BUG) messages
Replace all die("BUG: ...") calls by BUG() ones
run-command: use BUG() to report bugs, not die()
test-tool: help verifying BUG() code paths
|
|
In d8193743e08 (usage.c: add BUG() function, 2017-05-12), a new macro
was introduced to use for reporting bugs instead of die(). It was then
subsequently used to convert one single caller in 588a538ae55
(setup_git_env: convert die("BUG") to BUG(), 2017-05-12).
The cover letter of the patch series containing this patch
(cf 20170513032414.mfrwabt4hovujde2@sigill.intra.peff.net) is not
terribly clear why only one call site was converted, or what the plan
is for other, similar calls to die() to report bugs.
Let's just convert all remaining ones in one fell swoop.
This trick was performed by this invocation:
sed -i 's/die("BUG: /BUG("/g' $(git grep -l 'die("BUG' \*.c)
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
So far, we only ever asked whether stdout wants to be colorful. In the
upcoming patches, we will want to make push errors more prominent, which
are printed to stderr, though.
So let's refactor the want_color() function into a want_color_fd()
function (which expects to be called with fd == 1 or fd == 2 for stdout
and stderr, respectively), and then define the macro `want_color()` to
use the want_color_fd() function.
And then also add a macro `want_color_stderr()`, for convenience and
for documentation.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Add documentation explaining the functions in color.h.
While at it, migrate the function `color_set` into grep.c,
where the only callers are.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Move the code to detect "dumb" terminals into a single location. This
avoids duplicating the terminal detection code yet again in a subsequent
commit.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
This is the "theoretically more correct" approach of simply
stepping back to the state before plumbing commands started paying
attention to "color.ui" configuration variable.
Let's run with this one.
* jk/ref-filter-colors-fix:
tag: respect color.ui config
Revert "color: check color.ui in git_default_config()"
Revert "t6006: drop "always" color config tests"
Revert "color: make "always" the same as "auto" in config"
|
|
This reverts commit 136c8c8b8fa39f1315713248473dececf20f8fe7.
That commit was trying to address a bug caused by 4c7f1819b3
(make color.ui default to 'auto', 2013-06-10), in which
plumbing like diff-tree defaulted to "auto" color, but did
not respect a "color.ui" directive to disable it.
But it also meant that we started respecting "color.ui" set
to "always". This was a known problem, but 4c7f1819b3 argued
that nobody ought to be doing that. However, that turned out
to be wrong, and we got a number of bug reports related to
"add -p" regressing in v2.14.2.
Let's revert 136c8c8b8, fixing the regression to "add -p".
This leaves the problem from 4c7f1819b3 unfixed, but:
1. It's a pretty obscure problem in the first place. I
only noticed it while working on the color code, and we
haven't got a single bug report or complaint about it.
2. We can make a more moderate fix on top by respecting
"never" but not "always" for plumbing commands. This
is just the minimal fix to go back to the working state
we had before v2.14.2.
Note that this isn't a pure revert. We now have a test in
t3701 which shows off the "add -p" regression. This can be
flipped to success.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
This reverts commit 6be4595edb8e5b616c6e8b9fbc78b0f831fa2a87.
That commit weakened the "always" setting of color config so
that it acted as "auto". This was meant to solve regressions
in v2.14.2 in which setting "color.ui=always" in the on-disk
config broke scripts like add--interactive, because the
plumbing diff commands began to generate color output.
This was due to 136c8c8b8f (color: check color.ui in
git_default_config(), 2017-07-13), which was in turn trying
to fix issues caused by 4c7f1819b3 (make color.ui default to
'auto', 2013-06-10). But in weakening "always", we created
even more problems, as people expect to be able to use "git
-c color.ui=always" to force color (especially because some
commands don't have their own --color flag). We can fix that
by special-casing the command-line "-c", but now things are
getting pretty confusing.
Instead of piling hacks upon hacks, let's start peeling off
the hacks. The first step is dropping the weakening of
"always", which this revert does.
Note that we could actually revert the whole series merged
in by da15b78e52642bd45fd5513ab0000fdf2e58a6f4. Most of that
series consists of preparations to the tests to handle the
weakening of "-c color.ui=always". But it's worth keeping
for a few reasons:
- there are some other preparatory cleanups, like
e433749d86 (test-terminal: set TERM=vt100, 2017-10-03)
- it adds "--color" options more consistently in
0c88bf5050 (provide --color option for all ref-filter
users, 2017-10-03)
- some of the cases dropping "-c" end up being more robust
and realistic tests, as in 01c94e9001 (t7508: use
test_terminal for color output, 2017-10-03)
- the preferred tool for overriding config is "--color",
and we should be modeling that consistently
We can individually revert the few commits necessary to
restore some useful tests (which will be done on top of this
patch).
Note that this isn't a pure revert; we'll keep the test
added in t3701, but mark it as failure for now.
Signed-off-by: Jeff King <peff@peff.net>
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
|
|
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>
|
|
Assorted bugfixes and clean-ups.
* ma/ts-cleanups:
ThreadSanitizer: add suppressions
strbuf_setlen: don't write to strbuf_slopbuf
pack-objects: take lock before accessing `remaining`
convert: always initialize attr_action in convert_attrs
|
|
Add a file .tsan-suppressions and list two functions in it: want_color()
and transfer_debug(). Both of these use the pattern
static int foo = -1;
if (foo < 0)
foo = bar();
where bar always returns the same non-negative value. This can cause
ThreadSanitizer to diagnose a race when foo is written from two threads.
That is indeed a race, although it arguably doesn't matter in practice
since it's always the same value that is written.
Add NEEDSWORK-comments to the functions so that this problem is not
forever swept way under the carpet.
The suppressions-file is used by setting the environment variable
TSAN_OPTIONS to, e.g., "suppressions=$(pwd)/.tsan-suppressions". Observe
that relative paths such as ".tsan-suppressions" might not work.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Back in prehistoric times, our decision on whether or not to
show color by default relied on using a config callback that
either did or didn't load color config like color.diff.
When we introduced color.ui, we put it in the same boat:
commands had to manually respect it by using git_color_config()
or its git_color_default_config() convenience wrapper.
But in 4c7f1819b (make color.ui default to 'auto',
2013-06-10), that changed. Since then, we default color.ui
to auto in all programs, meaning that even plumbing commands
like "git diff-tree --pretty" might colorize the output.
Nobody seems to have complained in the intervening years,
presumably because the "is stdout a tty" check does a good
job of catching the right cases.
But that leaves an interesting curiosity: color.ui defaults
to auto even in plumbing, but you can't actually _disable_
the color via config. So if you really hate color and set
"color.ui" to false, diff-tree will still show color (but
porcelain like git-diff won't). Nobody noticed that either,
probably because very few people disable color.
One could argue that the plumbing should _always_ disable
color unless an explicit --color option is given on the
command line. But in practice, this creates a lot of
complications for scripts which do want plumbing to show
user-visible output. They can't just pass "--color" blindly;
they need to check the user's config and decide what to
send.
Given that nobody has complained about the current behavior,
let's assume it's a good path, and follow it to its
conclusion: supporting color.ui everywhere.
Note that you can create havoc by setting color.ui=always in
your config, but that's more or less already the case. We
could disallow it entirely, but it is handy for one-offs
like:
git -c color.ui=always foo >not-a-tty
when "foo" does not take a --color option itself.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Stop including config.h by default in cache.h. Instead only include
config.h in those files which require use of the config system.
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Some people feel the default set of colors used by "git log --graph"
rather limiting. A mechanism to customize the set of colors has
been introduced.
* nd/log-graph-configurable-colors:
document behavior of empty color name
color_parse_mem: allow empty color spec
log --graph: customize the graph lines with config log.graphColors
color.c: trim leading spaces in color_parse_mem()
color.c: fix color_parse_mem() with value_len == 0
|
|
Prior to c2f41bf52 (color.c: fix color_parse_mem() with
value_len == 0, 2017-01-19), the empty string was
interpreted as a color "reset". This was an accidental
outcome, and that commit turned it into an error.
However, scripts may pass the empty string as a default
value to "git config --get-color" to disable color when the
value is not defined. The git-add--interactive script does
this. As a result, the script is unusable since c2f41bf52
unless you have color.diff.plain defined (if it is defined,
then we don't parse the empty default at all).
Our test scripts didn't notice the recent breakage because
they run without a terminal, and thus without color. They
never hit this code path at all. And nobody noticed the
original buggy "reset" behavior, because it was effectively
a noop.
Let's fix the code to have an empty color name produce an
empty sequence of color codes. The tests need a few fixups:
- we'll add a new test in t4026 to cover this case. But
note that we need to tweak the color() helper. While
we're there, let's factor out the literal ANSI ESC
character. Otherwise it makes the diff quite hard to
read.
- we'll add a basic sanity-check in t4026 that "git add
-p" works at all when color is enabled. That would have
caught this bug, as well as any others that are specific
to the color code paths.
- 73c727d69 (log --graph: customize the graph lines with
config log.graphColors, 2017-01-19) added a test to
t4202 that checks some "invalid" graph color config.
Since ",, blue" before yielded only "blue" as valid, and
now yields "empty, empty, blue", we don't match the
expected output.
One way to fix this would be to change the expectation
to the empty color strings. But that makes the test much
less interesting, since we show only two graph lines,
both of which would be colorless.
Since the empty-string case is now covered by t4026,
let's remove them entirely here. They're just in the way
of the primary thing the test is supposed to be
checking.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Normally color_parse_mem() is called from config parser which trims the
leading spaces already. The new caller in the next patch won't. Let's be
tidy and trim leading spaces too (we already trim trailing spaces
after a word).
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
In this code we want to match the word "reset". If len is zero,
strncasecmp() will return zero and we incorrectly assume it's "reset" as
a result.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
* jk/squelch-false-warning-from-gcc-o3:
color_parse_mem: initialize "struct color" temporary
error_errno: use constant return similar to error()
|
|
Compiling color.c with gcc 6.2.0 using -O3 produces some
-Wmaybe-uninitialized false positives:
color.c: In function ‘color_parse_mem’:
color.c:189:10: warning: ‘bg.blue’ may be used uninitialized in this function [-Wmaybe-uninitialized]
out += xsnprintf(out, len, "%c8;2;%d;%d;%d", type,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
c->red, c->green, c->blue);
~~~~~~~~~~~~~~~~~~~~~~~~~~
color.c:208:15: note: ‘bg.blue’ was declared here
struct color bg = { COLOR_UNSPECIFIED };
^~
[ditto for bg.green, bg.red, fg.blue, etc]
This is doubly confusing, because the declaration shows it
being initialized! Even though we do not explicitly
initialize the color components, an incomplete initializer
sets the unmentioned members to zero.
What the warning doesn't show is that we later do this:
struct color c;
if (!parse_color(&c, ...)) {
if (fg.type == COLOR_UNSPECIFIED)
fg = c;
...
}
gcc is clever enough to realize that a struct assignment
from an uninitialized variable taints the destination. But
unfortunately it's _not_ clever enough to realize that we
only look at those members when type is set to COLOR_RGB, in
which case they are always initialized.
With -O2, gcc does not look into parse_color() and must
assume that "c" emerges fully initialized. With -O3, it
inlines parse_color(), and learns just enough to get
confused.
We can silence the false positive by initializing the
temporary "c". This also future-proofs us against
violating the type assumptions (the result would probably
still be buggy, but in a deterministic way).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The output coloring scheme learned two new attributes, italic and
strike, in addition to existing bold, reverse, etc.
* jk/ansi-color:
color: support strike-through attribute
color: support "italic" attribute
color: allow "no-" for negating attributes
color: refactor parse_attr
add skip_prefix_mem helper
doc: refactor description of color format
color: fix max-size comment
|
|
This is the only remaining attribute that is commonly
supported (at least by xterm) that we don't support. Let's
add it for completeness.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
We already support bold, underline, and similar attributes.
Let's add italic to the mix. According to the Wikipedia
page on ANSI colors, this attribute is "not widely
supported", but it does seem to work on my xterm.
We don't have to bump the maximum color size because we were
already over-allocating it (but we do adjust the comment
appropriately).
Requested-by: Simon Courtois <scourtois@cubyx.fr>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Using "no-bold" rather than "nobold" is easier to read and
more natural to type (to me, anyway, even though I was the
person who introduced "nobold" in the first place). It's
easy to allow both.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The list of attributes we recognize is a bit unwieldy, as we
actually have two arrays that must be kept in sync. Instead,
let's have a single array-of-struct to represent our
mapping. That means we can never have an accident that
causes us to read off the end of an array, and it makes
diffs for adding new attributes much easier to read.
This also makes it easy to handle the "no" cases without
having to repeat each attribute (this shortens the list,
making it easier to read, but also also cuts the size of our
linear search in half). Technically this makes it impossible
for us to add an attribute that starts with "no" (we could
confuse "nobody" for the negation of "body"), but since this
is a constrained set of attributes, that's OK.
Since we can also store the length of each name in the
struct, that makes it easy for us to avoid reading past the
"len" parameter given to us (though in practice it was not a
bug, since all of our current callers are interested in a
subset of a NUL-terminated buffer, not a true undelimited
range of memory).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
To set up default colors, we sometimes strcpy() from the
default string literals into our color buffers. This isn't a
bug (assuming the destination is COLOR_MAXLEN bytes), but
makes it harder to audit the code for problematic strcpy
calls.
Let's introduce a color_set which copies under the
assumption that there are COLOR_MAXLEN bytes in the
destination (of course you can call it on a smaller buffer,
so this isn't providing a huge amount of safety, but it's
more convenient than calling xsnprintf yourself).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Our color parsing is designed to never exceed COLOR_MAXLEN
bytes. But the relationship between that hand-computed
number and the parsing code is not at all obvious, and we
merely hope that it has been computed correctly for all
cases.
Let's mark the expected "end" pointer for the destination
buffer and make sure that we do not exceed it.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
