Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ui colors: Avoid white and use red for deleted status #3334

Merged
merged 6 commits into from Apr 16, 2019

Conversation

@kyleam
Copy link
Member

commented Apr 15, 2019

  • In spots where we want the output to use the default foreground color, stop coloring the output entirely rather than coloring it white. Using white leads to confusing output for terminals with white backgrounds.

  • In the datalad status output, paint the "deleted" status as red.


Note: The initial two commits are branched off of 0.11.x so that they can be merged into 0.11.x as well. (I'll take care of that.)

@kyleam

This comment has been minimized.

Copy link
Member Author

commented Apr 15, 2019

@kyleam kyleam added the WIP label Apr 15, 2019

kyleam added some commits Apr 15, 2019

ENH: ansi_colors: Add force parameter to color_word()
Make it possible to force the use of colors so that color_word() can
be used in log.ColorFormatter, which should obey the setting of its
use_color attribute.

Using color_word() in ColorFormatter arguably simplifies the code and
improves readability, and it will avoid repeating the conditional when
we start mapping a false color value to "no color".
ENH: ansi_colors: Support "default" color
The INFO level name is set to white in LOG_LEVEL_COLORS.  For
terminals with a dark background, this works as the default,
unremarkable color.  For terminals with a white background, the user
sees "[       ] ...".

Teach color_word() that a false value means "don't color", and change
INFO's color to None so that the default terminal color is used.
ENH: status: Use None, not white, as default color
The result renderer colors the state white if the state has not been
assigned a color in STATE_COLOR_MAP.  This is unfortunate for users
with a white terminal background.  A deleted file, for example, would
appear as

  $ datalad status
           : foo (file)

To instead use the default terminal foreground color, set the color to
None if the state isn't in the color map.  color_word() recently
learned to not paint the word if it receives a false value as the
color.
ENH: status: Paint status for deleted files red
Deleted files are a notable thing.  Give them more emphasis.

@kyleam kyleam force-pushed the kyleam:deleted-color branch from 25eccdd to 5038c40 Apr 15, 2019

@codecov

This comment has been minimized.

Copy link

commented Apr 15, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@d0a4772). Click here to learn what that means.
The diff coverage is 40%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #3334   +/-   ##
=========================================
  Coverage          ?   45.42%           
=========================================
  Files             ?      260           
  Lines             ?    34104           
  Branches          ?        0           
=========================================
  Hits              ?    15491           
  Misses            ?    18613           
  Partials          ?        0
Impacted Files Coverage Δ
datalad/core/local/status.py 93.75% <ø> (ø)
datalad/log.py 42.3% <0%> (ø)
datalad/support/ansi_colors.py 86.36% <50%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d0a4772...5038c40. Read the comment docs.

1 similar comment
@codecov

This comment has been minimized.

Copy link

commented Apr 15, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@d0a4772). Click here to learn what that means.
The diff coverage is 40%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #3334   +/-   ##
=========================================
  Coverage          ?   45.42%           
=========================================
  Files             ?      260           
  Lines             ?    34104           
  Branches          ?        0           
=========================================
  Hits              ?    15491           
  Misses            ?    18613           
  Partials          ?        0
Impacted Files Coverage Δ
datalad/core/local/status.py 93.75% <ø> (ø)
datalad/log.py 42.3% <0%> (ø)
datalad/support/ansi_colors.py 86.36% <50%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d0a4772...5038c40. Read the comment docs.

@kyleam kyleam removed the WIP label Apr 15, 2019

@kyleam

This comment has been minimized.

Copy link
Member Author

commented Apr 15, 2019

Need to look into a failing test: https://travis-ci.org/datalad/datalad/jobs/520325797#L1787

The issue was that log.ColorFormatter's use_color wasn't being respected. Fixed.

@kyleam

This comment has been minimized.

Copy link
Member Author

commented Apr 15, 2019

One appveyor run failed, but it looks to be unrelated: https://ci.appveyor.com/project/mih/datalad/builds/23864292/job/xccw0722uh4ch1hy#L817

@mih

mih approved these changes Apr 16, 2019

Copy link
Member

left a comment

LGTM, thx.

@mih mih merged commit e4096cd into datalad:master Apr 16, 2019

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
WIP Legacy commit status override — see details
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@kyleam kyleam deleted the kyleam:deleted-color branch Apr 16, 2019

kyleam added a commit that referenced this pull request Apr 16, 2019

Merge branch 'ansi-color-default' into 0.11.x
This brings in the 0.11.x component from gh-3334.

yarikoptic added a commit that referenced this pull request May 28, 2019

Merge tag '0.11.5' into debian
0.11.5 (May 23, 2019) -- stability is not overrated

Should be faster and less buggy, with a few enhancements.

 Fixes

- [create-sibling][]  ([#3318][])
  - Siblings are no longer configured with a post-update hook unless a
    web interface is requested with `--ui`.
  - `git submodule update --init` is no longer called from the
    post-update hook.
  - If `--inherit` is given for a dataset without a superdataset, a
    warning is now given instead of raising an error.
- The internal command runner failed on Python 2 when its `env`
  argument had unicode values.  ([#3332][])
- The safeguard that prevents creating a dataset in a subdirectory
  that already contains tracked files for another repository failed on
  Git versions before 2.14.  For older Git versions, we now warn the
  caller that the safeguard is not active.  ([#3347][])
- A regression introduced in v0.11.1 prevented [save][] from committing
  changes under a subdirectory when the subdirectory was specified as
  a path argument.  ([#3106][])
- A workaround introduced in v0.11.1 made it possible for [save][] to
  do a partial commit with an annex file that has gone below the
  `annex.largefiles` threshold.  The logic of this workaround was
  faulty, leading to files being displayed as typechanged in the index
  following the commit.  ([#3365][])
- The resolve_path() helper confused paths that had a semicolon for
  SSH RIs.  ([#3425][])
- The detection of SSH RIs has been improved.  ([#3425][])

 Enhancements and new features

- The internal command runner was too aggressive in its decision to
  sleep.  ([#3322][])
- The "INFO" label in log messages now retains the default text color
  for the terminal rather than using white, which only worked well for
  terminals with dark backgrounds.  ([#3334][])
- A short flag `-R` is now available for the `--recursion-limit` flag,
  a flag shared by several subcommands.  ([#3340][])
- The authentication logic for [create-sibling-github][] has been
  revamped and now supports 2FA.  ([#3180][])
- New configuration option `datalad.ui.progressbar` can be used to
  configure the default backend for progress reporting ("none", for
  example, results in no progress bars being shown).  ([#3396][])
- A new progress backend, available by setting datalad.ui.progressbar
  to "log", replaces progress bars with a log message upon completion
  of an action.  ([#3396][])
- DataLad learned to consult the [NO_COLOR][] environment variable and
  the new `datalad.ui.color` configuration option when deciding to
  color output.  The default value, "auto", retains the current
  behavior of coloring output if attached to a TTY ([#3407][]).
- [clean][] now removes annex transfer directories, which is useful
  for cleaning up failed downloads. ([#3374][])
- [clone][] no longer refuses to clone into a local path that looks
  like a URL, making its behavior consistent with `git clone`.
  ([#3425][])
- [wtf][]
  - Learned to fall back to the `dist` package if `platform.dist`,
    which has been removed in the yet-to-be-release Python 3.8, does
    not exist.  ([#3439][])
  - Gained a `--section` option for limiting the output to specific
    sections and a `--decor` option, which currently knows how to
    format the output as GitHub's `<details>` section.  ([#3440][])

* tag '0.11.5': (96 commits)
  [DATALAD RUNCMD] make update-changelog
  Version boost and finalize CHANGELOG.md record
  ENH: new Makefile rule linkissues-changelog to link issues, which now will also be prerequisite for update-changelog
  CHANGELOG.md: Add entries for recently merged PRs
  ENH: require "distro" for python >= 3.8
  ENH: compat with python 3.8 which removed .dist -- try distro
  CLN: wtf: Remove unused (and duplicated) import
  DOC: wtf: Avoid double period in -S's description
  ENH: -D|--decor html_details -- to make it ready for pasting to github issue without clutter
  BF: assure bytes while giving to pyperclip upon its demand (on Py2)
  RF: move always present path + type "section" into "location" section, retain order of sections from cmdline
  RF: switch from nargs="*" to action=append for wtf -S
  ENH: wtf -S to specify which sections to query/display (by default -- all)
  MNT: Avoid invalid escape sequences in strings
  BF: export_to_figshare: Don't test identity of string literal
  BF(TST): do not assume user naiveness - treat any url-like looking path as a path
  BF: Check for /, \ or # in the username@hostname part while detecting SSHRI
  CHANGELOG.md: Add entry for gh-3374
  BF: revert back (remove) check for path being PathRI
  BF: list annex-transfer also in cmdline opt choice for "what"
  ...

yarikoptic added a commit that referenced this pull request May 28, 2019

Merge tag '0.11.5' into debian -- got two more minor changes
0.11.5 (May 23, 2019) -- stability is not overrated

Should be faster and less buggy, with a few enhancements.

 Fixes

- [create-sibling][]  ([#3318][])
  - Siblings are no longer configured with a post-update hook unless a
    web interface is requested with `--ui`.
  - `git submodule update --init` is no longer called from the
    post-update hook.
  - If `--inherit` is given for a dataset without a superdataset, a
    warning is now given instead of raising an error.
- The internal command runner failed on Python 2 when its `env`
  argument had unicode values.  ([#3332][])
- The safeguard that prevents creating a dataset in a subdirectory
  that already contains tracked files for another repository failed on
  Git versions before 2.14.  For older Git versions, we now warn the
  caller that the safeguard is not active.  ([#3347][])
- A regression introduced in v0.11.1 prevented [save][] from committing
  changes under a subdirectory when the subdirectory was specified as
  a path argument.  ([#3106][])
- A workaround introduced in v0.11.1 made it possible for [save][] to
  do a partial commit with an annex file that has gone below the
  `annex.largefiles` threshold.  The logic of this workaround was
  faulty, leading to files being displayed as typechanged in the index
  following the commit.  ([#3365][])
- The resolve_path() helper confused paths that had a semicolon for
  SSH RIs.  ([#3425][])
- The detection of SSH RIs has been improved.  ([#3425][])

 Enhancements and new features

- The internal command runner was too aggressive in its decision to
  sleep.  ([#3322][])
- The "INFO" label in log messages now retains the default text color
  for the terminal rather than using white, which only worked well for
  terminals with dark backgrounds.  ([#3334][])
- A short flag `-R` is now available for the `--recursion-limit` flag,
  a flag shared by several subcommands.  ([#3340][])
- The authentication logic for [create-sibling-github][] has been
  revamped and now supports 2FA.  ([#3180][])
- New configuration option `datalad.ui.progressbar` can be used to
  configure the default backend for progress reporting ("none", for
  example, results in no progress bars being shown).  ([#3396][])
- A new progress backend, available by setting datalad.ui.progressbar
  to "log", replaces progress bars with a log message upon completion
  of an action.  ([#3396][])
- DataLad learned to consult the [NO_COLOR][] environment variable and
  the new `datalad.ui.color` configuration option when deciding to
  color output.  The default value, "auto", retains the current
  behavior of coloring output if attached to a TTY ([#3407][]).
- [clean][] now removes annex transfer directories, which is useful
  for cleaning up failed downloads. ([#3374][])
- [clone][] no longer refuses to clone into a local path that looks
  like a URL, making its behavior consistent with `git clone`.
  ([#3425][])
- [wtf][]
  - Learned to fall back to the `dist` package if `platform.dist`,
    which has been removed in the yet-to-be-release Python 3.8, does
    not exist.  ([#3439][])
  - Gained a `--section` option for limiting the output to specific
    sections and a `--decor` option, which currently knows how to
    format the output as GitHub's `<details>` section.  ([#3440][])

* tag '0.11.5':
  BF: make test for url download more reliable in cases where connection fails
  RF: remove stale commented out duecredit in setup.py.  It has now its own section
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.