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

Change option --color choices to auto/always/never #1701

Merged
merged 13 commits into from Sep 5, 2016
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions easybuild/main.py
Expand Up @@ -56,15 +56,14 @@
from easybuild.tools.config import find_last_log, get_repository, get_repositorypath, build_option
from easybuild.tools.filetools import adjust_permissions, cleanup, write_file
from easybuild.tools.github import check_github, install_github_token, new_pr, update_pr
from easybuild.tools.options import parse_external_modules_metadata, process_software_build_specs
from easybuild.tools.options import parse_external_modules_metadata, process_software_build_specs, use_color
from easybuild.tools.robot import det_robot_path, dry_run, resolve_dependencies, search_easyconfigs
from easybuild.tools.package.utilities import check_pkg_support
from easybuild.tools.parallelbuild import submit_jobs
from easybuild.tools.repository.repository import init_repository
from easybuild.tools.testing import create_test_report, overall_test_report, regtest, session_module_list, session_state
from easybuild.tools.version import this_is_easybuild


_log = None


Expand Down Expand Up @@ -171,7 +170,7 @@ def handle_github_options(options, ec_paths):
new_pr(ec_paths, title=options.pr_title, descr=options.pr_descr, commit_msg=options.pr_commit_msg)

elif options.review_pr:
print review_pr(options.review_pr, colored=options.color)
print review_pr(options.review_pr, colored=use_color(options.color))

elif options.update_pr:
update_pr(options.update_pr, ec_paths, commit_msg=options.pr_commit_msg)
Expand Down Expand Up @@ -208,7 +207,8 @@ def main(args=None, logfile=None, do_build=None, testing=False):

# initialise logging for main
global _log
_log, logfile = init_logging(logfile, logtostdout=options.logtostdout, silent=testing or options.terse)
_log, logfile = init_logging(logfile, logtostdout=options.logtostdout,
silent=(testing or options.terse), colorize=options.color)

# disallow running EasyBuild as root
if os.getuid() == 0:
Expand Down
4 changes: 2 additions & 2 deletions easybuild/tools/build_log.py
Expand Up @@ -186,10 +186,10 @@ def exception(self, msg, *args):
_init_easybuildlog = fancylogger.getLogger(fname=False)


def init_logging(logfile, logtostdout=False, silent=False):
def init_logging(logfile, logtostdout=False, silent=False, colorize='auto'):
"""Initialize logging."""
if logtostdout:
fancylogger.logToScreen(enable=True, stdout=True)
fancylogger.logToScreen(enable=True, stdout=True, colorize=colorize)
else:
if logfile is None:
# mkstemp returns (fd,filename), fd is from os.open, not regular open!
Expand Down
31 changes: 30 additions & 1 deletion easybuild/tools/options.py
Expand Up @@ -80,6 +80,17 @@
from vsc.utils import fancylogger
from vsc.utils.generaloption import GeneralOption

try:
from humanfriendly.terminal import terminal_supports_colors
except ImportError:
# provide an approximation that should work in most cases
def terminal_supports_colors(stream):
try:
return os.isatty(stream.fileno())
except Exception:
# in case of errors do not bother and just return the safe default
return False


CONFIG_ENV_VAR_PREFIX = 'EASYBUILD'

Expand Down Expand Up @@ -142,6 +153,24 @@ def pretty_print_opts(opts_dict):
print '\n'.join(lines)


def use_color(colorize, stream=sys.stdout):
"""
Return ``True`` or ``False`` depending on whether ANSI color
escapes are to be used when printing to `stream`.

The `colorize` argument can take the three string values
``'auto'``/``'always'``/``'never'``, see the ``--color`` option
for their meaning.
"""
# turn color=auto/yes/no into a boolean value
if options.color == 'auto':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@riccardomurri this should use colorize rather than options.color?

return terminal_supports_colors(stream)
elif options.color == 'always':
return True
else:
return False


class EasyBuildOptions(GeneralOption):
"""Easybuild generaloption class"""
VERSION = this_is_easybuild()
Expand Down Expand Up @@ -258,7 +287,7 @@ def override_options(self):
None, 'store_true', False),
'cleanup-builddir': ("Cleanup build dir after successful installation.", None, 'store_true', True),
'cleanup-tmpdir': ("Cleanup tmp dir after successful run.", None, 'store_true', True),
'color': ("Allow color output", None, 'store_true', True),
'color': ("Colorize output", 'choice', 'store', 'auto', ['auto', 'always', 'never'], {'metavar':'WHEN'}),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since enabling this requires having humanfriendly installed, I wouldn't enable this by default...

also, changing this may break people's configuration files, since they may specify False or True now, so we'll need to be careful with changing this to string, and at least translate False to never and True to auto (I wouldn't go with always since using that is probably rare...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Kenneth Hoste, Fri, Mar 25, 2016 at 03:44:00PM -0700:)

@@ -258,7 +258,7 @@ def override_options(self):
None, 'store_true', False),
'cleanup-builddir': ("Cleanup build dir after successful installation.", None, 'store_true', True),
'cleanup-tmpdir': ("Cleanup tmp dir after successful run.", None, 'store_true', True),

  •        'color': ("Allow color output", None, 'store_true', True),
    
  •        'color': ("Colorize output", 'choice', 'store', 'auto', ['auto', 'always', 'never'], {'metavar':'WHEN'}),
    

since enabling this requires having humanfriendly installed,

No it doesn't -- if you don't have the required packages, it will
silently fall back to "no colorization".

also, changing this may break people's configuration files, since they
may specify False or True now, so we'll need to be careful with
changing this to string, and at least translate False to never and
True to auto

Good point, but how to I do that with vsc's option parsing framework?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, changing this may break people's configuration files, since they
may specify False or True now, so we'll need to be careful with
changing this to string, and at least translate False to never and
True to auto

Good point, but how to I do that with vsc's option parsing framework?

Actually, I would translate the other way round:

  • True -> auto
  • False -> never

'default-opt-level': ("Specify default optimisation level", 'choice', 'store', DEFAULT_OPT_LEVEL,
Compiler.COMPILER_OPT_FLAGS),
'deprecated': ("Run pretending to be (future) version, to test removal of deprecated code.",
Expand Down
5 changes: 5 additions & 0 deletions setup.py
Expand Up @@ -116,6 +116,11 @@ def find_rel_test():
],
extras_require = {
'yeb': ["PyYAML >= 3.11"],
'coloredlogs': [
'vsc-base[coloredlogs] >= 2.5.0',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@riccardomurri please bump to 2.5.3 as minimal required version

'coloredlogs',
'humanfriendly', # determine whether terminal supports ANSI color
],
},
namespace_packages=['easybuild'],
)
8 changes: 4 additions & 4 deletions test/framework/options.py
Expand Up @@ -63,7 +63,7 @@
name = foo, bar
version = 1.2.3, 3.2.1
prefix = FOOBAR_DIR

[foobar/2.0]
name = foobar
version = 2.0
Expand Down Expand Up @@ -217,8 +217,8 @@ def test_force(self):

# clear log file
write_file(self.logfile, '')
# check that --force and --rebuild work

# check that --force and --rebuild work
for arg in ['--force', '--rebuild']:
outtxt = self.eb_main([eb_file, '--debug', arg])
self.assertTrue(not re.search(already_msg, outtxt), "Already installed message not there with %s" % arg)
Expand Down Expand Up @@ -2050,7 +2050,7 @@ def test_review_pr(self):
"""Test --review-pr."""
self.mock_stdout(True)
# PR for zlib 1.2.8 easyconfig, see https://github.com/hpcugent/easybuild-easyconfigs/pull/1484
self.eb_main(['--review-pr=1484', '--disable-color'], raise_error=True)
self.eb_main(['--review-pr=1484', '--color=never'], raise_error=True)
txt = self.get_stdout()
self.mock_stdout(False)
self.assertTrue(re.search(r"^Comparing zlib-1.2.8\S* with zlib-1.2.8", txt))
Expand Down