From 696648d18961586280b3ecee4cf05b83be204be4 Mon Sep 17 00:00:00 2001 From: Skip Montanaro Date: Thu, 15 Feb 2024 10:40:59 -0600 Subject: [PATCH 1/9] suggested color changes for cherry-picker output --- cherry_picker/cherry_picker.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/cherry_picker/cherry_picker.py b/cherry_picker/cherry_picker.py index f2046ee..0cb410c 100755 --- a/cherry_picker/cherry_picker.py +++ b/cherry_picker/cherry_picker.py @@ -306,7 +306,7 @@ def cherry_pick(self): click.echo(self.run_cmd(cmd)) except subprocess.CalledProcessError as err: click.echo(f"Error cherry-pick {self.commit_sha1}.") - click.echo(err.output) + click.secho(err.output, fg=(128, 128, 128)) raise CherryPickException(f"Error cherry-pick {self.commit_sha1}.") def get_exit_message(self, branch): @@ -387,7 +387,7 @@ def amend_commit_message(self, cherry_pick_branch): return updated_commit_message def pause_after_committing(self, cherry_pick_branch): - click.echo( + click.secho( f""" Finished cherry-pick {self.commit_sha1} into {cherry_pick_branch} \U0001F600 --no-push option used. @@ -397,7 +397,8 @@ def pause_after_committing(self, cherry_pick_branch): To abort the cherry-pick and cleanup: $ cherry_picker --abort -""" +""", + fg="red" ) self.set_paused_state() @@ -468,8 +469,8 @@ def open_pr(self, url): if self.dry_run: click.echo(f" dry-run: Create new PR: {url}") else: - click.echo("Backport PR URL:") - click.echo(url) + click.secho("Backport PR URL:", fg="red") + click.secho(url, fg="red") webbrowser.open_new_tab(url) def delete_branch(self, branch): @@ -530,9 +531,9 @@ def backport(self): commit_message = self.amend_commit_message(cherry_pick_branch) except subprocess.CalledProcessError as cpe: click.echo(cpe.output) - click.echo(self.get_exit_message(maint_branch)) + click.secho(self.get_exit_message(maint_branch), fg="red") except CherryPickException: - click.echo(self.get_exit_message(maint_branch)) + click.secho(self.get_exit_message(maint_branch), fg="red") self.set_paused_state() raise else: From f79b0db287f364e3192a6414ab037da8de99b98d Mon Sep 17 00:00:00 2001 From: Skip Montanaro Date: Thu, 15 Feb 2024 10:55:46 -0600 Subject: [PATCH 2/9] working around lint complaint --- cherry_picker/cherry_picker.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cherry_picker/cherry_picker.py b/cherry_picker/cherry_picker.py index 0cb410c..80575aa 100755 --- a/cherry_picker/cherry_picker.py +++ b/cherry_picker/cherry_picker.py @@ -398,8 +398,7 @@ def pause_after_committing(self, cherry_pick_branch): To abort the cherry-pick and cleanup: $ cherry_picker --abort """, - fg="red" - ) + fg="red") self.set_paused_state() def push_to_remote(self, base_branch, head_branch, commit_message=""): From c8760ba63fa4e530b29ebb265e490db9abbdb852 Mon Sep 17 00:00:00 2001 From: Skip Montanaro Date: Thu, 15 Feb 2024 10:57:45 -0600 Subject: [PATCH 3/9] fine. you want a trailing comma and newline in an argument list? you got it. --- cherry_picker/cherry_picker.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cherry_picker/cherry_picker.py b/cherry_picker/cherry_picker.py index 80575aa..3ef431d 100755 --- a/cherry_picker/cherry_picker.py +++ b/cherry_picker/cherry_picker.py @@ -398,7 +398,8 @@ def pause_after_committing(self, cherry_pick_branch): To abort the cherry-pick and cleanup: $ cherry_picker --abort """, - fg="red") + fg="red", + ) self.set_paused_state() def push_to_remote(self, base_branch, head_branch, commit_message=""): From d24cf6d4b0e967065b2622d6375dd3bead859077 Mon Sep 17 00:00:00 2001 From: Skip Montanaro Date: Thu, 15 Feb 2024 20:25:53 -0600 Subject: [PATCH 4/9] support NO_COLOR, and --color/--no-color --- cherry_picker/cherry_picker.py | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/cherry_picker/cherry_picker.py b/cherry_picker/cherry_picker.py index 3ef431d..995bee8 100755 --- a/cherry_picker/cherry_picker.py +++ b/cherry_picker/cherry_picker.py @@ -109,6 +109,7 @@ def __init__( config=DEFAULT_CONFIG, chosen_config_path=None, auto_pr=True, + use_color=True, ): self.chosen_config_path = chosen_config_path """The config reference used in the current runtime. @@ -138,6 +139,13 @@ def __init__( self.auto_pr = auto_pr self.prefix_commit = prefix_commit + if use_color and os.environ.get("NO_COLOR") is None: + self.dim = (128, 128, 128) + self.bright = "red" + else: + self.dim = "black" + self.bright = "black" + # the cached calculated value of self.upstream property self._upstream = None @@ -306,7 +314,7 @@ def cherry_pick(self): click.echo(self.run_cmd(cmd)) except subprocess.CalledProcessError as err: click.echo(f"Error cherry-pick {self.commit_sha1}.") - click.secho(err.output, fg=(128, 128, 128)) + click.secho(err.output, fg=self.dim) raise CherryPickException(f"Error cherry-pick {self.commit_sha1}.") def get_exit_message(self, branch): @@ -398,7 +406,7 @@ def pause_after_committing(self, cherry_pick_branch): To abort the cherry-pick and cleanup: $ cherry_picker --abort """, - fg="red", + fg=self.bright, ) self.set_paused_state() @@ -469,8 +477,8 @@ def open_pr(self, url): if self.dry_run: click.echo(f" dry-run: Create new PR: {url}") else: - click.secho("Backport PR URL:", fg="red") - click.secho(url, fg="red") + click.secho("Backport PR URL:", fg=self.bright) + click.secho(url, fg=self.bright) webbrowser.open_new_tab(url) def delete_branch(self, branch): @@ -531,9 +539,9 @@ def backport(self): commit_message = self.amend_commit_message(cherry_pick_branch) except subprocess.CalledProcessError as cpe: click.echo(cpe.output) - click.secho(self.get_exit_message(maint_branch), fg="red") + click.secho(self.get_exit_message(maint_branch), fg=self.bright) except CherryPickException: - click.secho(self.get_exit_message(maint_branch), fg="red") + click.secho(self.get_exit_message(maint_branch), fg=self.bright) self.set_paused_state() raise else: @@ -777,6 +785,16 @@ def is_mirror(self) -> bool: ) @click.argument("commit_sha1", nargs=1, default="") @click.argument("branches", nargs=-1) +@click.option( + "--color/--no-color", + "color", + is_flag=True, + default=True, + help=( + "If color display is enabled, cherry-picker will use color to distinguish" + " its output from that of 'git cherry-pick'." + ), +) @click.pass_context def cherry_pick_cli( ctx, @@ -790,6 +808,7 @@ def cherry_pick_cli( config_path, commit_sha1, branches, + color, ): """cherry-pick COMMIT_SHA1 into target BRANCHES.""" @@ -808,6 +827,7 @@ def cherry_pick_cli( auto_pr=auto_pr, config=config, chosen_config_path=chosen_config_path, + use_color=color, ) except InvalidRepoException: click.echo(f"You're not inside a {config['repo']} repo right now! \U0001F645") From a9ca1eafc5a1c4cc845a69a6506391bee9f6ae9a Mon Sep 17 00:00:00 2001 From: Skip Montanaro Date: Fri, 23 Feb 2024 15:19:31 -0600 Subject: [PATCH 5/9] support bright and dim colors in multiple forms --- cherry_picker/cherry_picker.py | 95 +++++++++++++++++++++++++++++++--- 1 file changed, 88 insertions(+), 7 deletions(-) diff --git a/cherry_picker/cherry_picker.py b/cherry_picker/cherry_picker.py index 995bee8..8083238 100755 --- a/cherry_picker/cherry_picker.py +++ b/cherry_picker/cherry_picker.py @@ -77,6 +77,8 @@ """, ) +BRIGHT = "red" +DIM = "0x808080" class BranchCheckoutException(Exception): def __init__(self, branch_name): @@ -110,6 +112,8 @@ def __init__( chosen_config_path=None, auto_pr=True, use_color=True, + bright=None, + dim=None, ): self.chosen_config_path = chosen_config_path """The config reference used in the current runtime. @@ -138,13 +142,15 @@ def __init__( self.push = push self.auto_pr = auto_pr self.prefix_commit = prefix_commit + self.dim = None + self.bright = None if use_color and os.environ.get("NO_COLOR") is None: - self.dim = (128, 128, 128) - self.bright = "red" - else: - self.dim = "black" - self.bright = "black" + self.dim = DIM if dim is None else dim + self.bright = BRIGHT if bright is None else bright + + self.dim = normalize_color(self.dim) + self.bright = normalize_color(self.bright) # the cached calculated value of self.upstream property self._upstream = None @@ -791,8 +797,27 @@ def is_mirror(self) -> bool: is_flag=True, default=True, help=( - "If color display is enabled, cherry-picker will use color to distinguish" - " its output from that of 'git cherry-pick'." + "If color display is enabled (the default), cherry-picker will use color to" + " distinguish its output from that of 'git cherry-pick'. Color display can" + " also be suppressed by setting NO_COLOR environment variable to non-empty value." + ), +) +@click.option( + "--bright", + "bright", + default=BRIGHT, + help=( + "Foreground color for bright text (from cherry_picker). Can be color name," + " RGB hex string ('0xRRGGBB' or '0xRGB') or tuple of ints ('44, 88, 255)'" + ), +) +@click.option( + "--dim", + "dim", + default=DIM, + help=( + "Foreground color for dim text (from git cherry-pick). Can be color name," + " RGB hex string ('0xRRGGBB' or '0xRGB') or tuple of ints ('44, 88, 255)'" ), ) @click.pass_context @@ -809,6 +834,8 @@ def cherry_pick_cli( commit_sha1, branches, color, + bright, + dim, ): """cherry-pick COMMIT_SHA1 into target BRANCHES.""" @@ -828,6 +855,8 @@ def cherry_pick_cli( config=config, chosen_config_path=chosen_config_path, use_color=color, + bright=bright, + dim=dim, ) except InvalidRepoException: click.echo(f"You're not inside a {config['repo']} repo right now! \U0001F645") @@ -1092,5 +1121,57 @@ def get_state_from_string(state_str): return WORKFLOW_STATES.__members__[state_str] +def normalize_color(color): + """Produce colors in Click's format from strings. + + Supported formats are: + + * color names understood by Click. See: + https://click.palletsprojects.com/en/8.1.x/api/#click.style + * RGB values of the form 0xRRGGBB or 0xRGB. + * Tuple of ints (base 10), must have leading and trailing parens, + and values separated by commas, e.g., "( 44, 88, 255)". + + """ + + if color is None: + return color + + color = color.strip().lower() + + if color.lower().startswith("0x"): + # Assume it's a hex string and convert it to Click's tuple of ints + # form. It's kind of surprising Click doesn't support hex strings + # directly. + match len(color): + case 8: + # assume six-digit hex, 0xRRGGBB + return ( + int(color[2:4], 16), + int(color[4:6], 16), + int(color[6:8], 16), + ) + case 5: + # assume three-digit hex, 0xRGB + return ( + int(color[2], 16) * 16, + int(color[3], 16) * 16, + int(color[4], 16) * 16, + ) + case _: + raise ValueError(f"unrecognized hex string: {color}") + + if color[0] == "(" and color[-1] == ")": + # let's hope it's a tuple of ints... + try: + red, green, blue = [int(x.strip()) for x in color[1:-1].split(",")] + except ValueError as exc: + raise ValueError(f"unrecognized tuple of ints string: {color}") from exc + return (red, green, blue) + + # fallback is to assume it's a color name supported by click. + return color + + if __name__ == "__main__": cherry_pick_cli() From b33029e1cbce073bb61cc1b45e7cad52212d8f1f Mon Sep 17 00:00:00 2001 From: Skip Montanaro Date: Sat, 24 Feb 2024 08:42:55 -0600 Subject: [PATCH 6/9] click *does* have a color list! --- cherry_picker/cherry_picker.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cherry_picker/cherry_picker.py b/cherry_picker/cherry_picker.py index 8083238..1b80a14 100755 --- a/cherry_picker/cherry_picker.py +++ b/cherry_picker/cherry_picker.py @@ -11,6 +11,7 @@ import webbrowser import click +from click.termui import _ansi_colors as color_names import requests from gidgethub import sansio @@ -1170,7 +1171,9 @@ def normalize_color(color): return (red, green, blue) # fallback is to assume it's a color name supported by click. - return color + if color in color_names: + return color + raise ValueError(f"unrecognized color: '{color}'") if __name__ == "__main__": From d2edb03b7a00232d3d6800e68e30203be3f89037 Mon Sep 17 00:00:00 2001 From: Skip Montanaro Date: Sun, 10 Mar 2024 16:26:49 -0500 Subject: [PATCH 7/9] use repr for color spec errors --- cherry_picker/cherry_picker.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cherry_picker/cherry_picker.py b/cherry_picker/cherry_picker.py index 1b80a14..02ed237 100755 --- a/cherry_picker/cherry_picker.py +++ b/cherry_picker/cherry_picker.py @@ -1160,20 +1160,20 @@ def normalize_color(color): int(color[4], 16) * 16, ) case _: - raise ValueError(f"unrecognized hex string: {color}") + raise ValueError(f"unrecognized hex string: {color!r}") if color[0] == "(" and color[-1] == ")": # let's hope it's a tuple of ints... try: red, green, blue = [int(x.strip()) for x in color[1:-1].split(",")] except ValueError as exc: - raise ValueError(f"unrecognized tuple of ints string: {color}") from exc + raise ValueError(f"unrecognized tuple of ints string: {color!r}") from exc return (red, green, blue) # fallback is to assume it's a color name supported by click. if color in color_names: return color - raise ValueError(f"unrecognized color: '{color}'") + raise ValueError(f"unrecognized color: '{color!r}'") if __name__ == "__main__": From cb4e1e7b008a6fa2a59c6e2f56ba1e382b47e2a0 Mon Sep 17 00:00:00 2001 From: Skip Montanaro Date: Sun, 10 Mar 2024 16:28:17 -0500 Subject: [PATCH 8/9] cherry picker goes back to 3.8, so match stmt can't be used --- cherry_picker/cherry_picker.py | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/cherry_picker/cherry_picker.py b/cherry_picker/cherry_picker.py index 02ed237..c3ea132 100755 --- a/cherry_picker/cherry_picker.py +++ b/cherry_picker/cherry_picker.py @@ -1144,23 +1144,22 @@ def normalize_color(color): # Assume it's a hex string and convert it to Click's tuple of ints # form. It's kind of surprising Click doesn't support hex strings # directly. - match len(color): - case 8: - # assume six-digit hex, 0xRRGGBB - return ( - int(color[2:4], 16), - int(color[4:6], 16), - int(color[6:8], 16), - ) - case 5: - # assume three-digit hex, 0xRGB - return ( - int(color[2], 16) * 16, - int(color[3], 16) * 16, - int(color[4], 16) * 16, - ) - case _: - raise ValueError(f"unrecognized hex string: {color!r}") + if len(color) == 8: + # assume six-digit hex, 0xRRGGBB + return ( + int(color[2:4], 16), + int(color[4:6], 16), + int(color[6:8], 16), + ) + elif len(color) == 5: + # assume three-digit hex, 0xRGB + return ( + int(color[2], 16) * 16, + int(color[3], 16) * 16, + int(color[4], 16) * 16, + ) + else: + raise ValueError(f"unrecognized hex string: {color!r}") if color[0] == "(" and color[-1] == ")": # let's hope it's a tuple of ints... From fde4186ea936fabc73a782b3ef7442b003af9eb5 Mon Sep 17 00:00:00 2001 From: Skip Montanaro Date: Mon, 11 Mar 2024 09:31:01 -0500 Subject: [PATCH 9/9] turn bright and dim into arguments instead of options --- cherry_picker/cherry_picker.py | 38 ++++++----------------------- cherry_picker/test_cherry_picker.py | 6 +++++ 2 files changed, 14 insertions(+), 30 deletions(-) diff --git a/cherry_picker/cherry_picker.py b/cherry_picker/cherry_picker.py index c3ea132..2ff823b 100755 --- a/cherry_picker/cherry_picker.py +++ b/cherry_picker/cherry_picker.py @@ -32,6 +32,8 @@ "check_sha": "7f777ed95a19224294949e1b4ce56bbffcb1fe9f", "fix_commit_msg": True, "default_branch": "main", + "bright": "red", + "dim": "0x808080", } ) @@ -78,9 +80,6 @@ """, ) -BRIGHT = "red" -DIM = "0x808080" - class BranchCheckoutException(Exception): def __init__(self, branch_name): self.branch_name = branch_name @@ -113,8 +112,6 @@ def __init__( chosen_config_path=None, auto_pr=True, use_color=True, - bright=None, - dim=None, ): self.chosen_config_path = chosen_config_path """The config reference used in the current runtime. @@ -143,12 +140,11 @@ def __init__( self.push = push self.auto_pr = auto_pr self.prefix_commit = prefix_commit - self.dim = None - self.bright = None + self.dim = config["dim"] + self.bright = config["bright"] - if use_color and os.environ.get("NO_COLOR") is None: - self.dim = DIM if dim is None else dim - self.bright = BRIGHT if bright is None else bright + if not use_color or os.environ.get("NO_COLOR") is not None: + self.dim = self.bright = None self.dim = normalize_color(self.dim) self.bright = normalize_color(self.bright) @@ -803,24 +799,8 @@ def is_mirror(self) -> bool: " also be suppressed by setting NO_COLOR environment variable to non-empty value." ), ) -@click.option( - "--bright", - "bright", - default=BRIGHT, - help=( - "Foreground color for bright text (from cherry_picker). Can be color name," - " RGB hex string ('0xRRGGBB' or '0xRGB') or tuple of ints ('44, 88, 255)'" - ), -) -@click.option( - "--dim", - "dim", - default=DIM, - help=( - "Foreground color for dim text (from git cherry-pick). Can be color name," - " RGB hex string ('0xRRGGBB' or '0xRGB') or tuple of ints ('44, 88, 255)'" - ), -) +@click.argument("bright", nargs=1) +@click.argument("dim", nargs=1) @click.pass_context def cherry_pick_cli( ctx, @@ -856,8 +836,6 @@ def cherry_pick_cli( config=config, chosen_config_path=chosen_config_path, use_color=color, - bright=bright, - dim=dim, ) except InvalidRepoException: click.echo(f"You're not inside a {config['repo']} repo right now! \U0001F645") diff --git a/cherry_picker/test_cherry_picker.py b/cherry_picker/test_cherry_picker.py index 5138c83..78e4bec 100644 --- a/cherry_picker/test_cherry_picker.py +++ b/cherry_picker/test_cherry_picker.py @@ -456,6 +456,8 @@ def test_load_full_config(tmp_git_repo_dir, git_add, git_commit): "team": "python", "fix_commit_msg": True, "default_branch": "devel", + "bright": "red", + "dim": "0x808080", }, ) @@ -479,6 +481,8 @@ def test_load_partial_config(tmp_git_repo_dir, git_add, git_commit): "team": "python", "fix_commit_msg": True, "default_branch": "main", + "bright": "red", + "dim": "0x808080", }, ) @@ -507,6 +511,8 @@ def test_load_config_no_head_sha(tmp_git_repo_dir, git_add, git_commit): "team": "python", "fix_commit_msg": True, "default_branch": "devel", + "bright": "red", + "dim": "0x808080", }, )