From de36bc6b7ad4eb57e61e67237b998046a60f852d Mon Sep 17 00:00:00 2001 From: Sean Khan Date: Thu, 15 May 2025 13:00:02 -0400 Subject: [PATCH 1/2] refactor: Use f-strings for string formatting Since this project has a minimum requirement of Python 3.8, we can safely use f-strings for string formatting instead of the older `%` formatting method. This change improves readability and performance, as f-strings are generally more efficient than the older methods. Signed-off-by: Sean Khan --- pwclient/api.py | 20 +++++++++----------- pwclient/checks.py | 6 +++--- pwclient/patches.py | 12 ++++++------ pwclient/shell.py | 23 +++++++++++------------ pwclient/utils.py | 6 +++--- pwclient/xmlrpc.py | 6 +++--- 6 files changed, 35 insertions(+), 38 deletions(-) diff --git a/pwclient/api.py b/pwclient/api.py index 15bceb8..79d1d28 100644 --- a/pwclient/api.py +++ b/pwclient/api.py @@ -269,8 +269,7 @@ def patch_list( state_id = self._state_id_by_name(state) if state_id == 0: sys.stderr.write( - "Note: No State found matching %s*, " - "ignoring filter\n" % state + f"Note: No State found matching {state}*, ignoring filter\n" ) else: filters['state_id'] = state_id @@ -279,8 +278,7 @@ def patch_list( project_id = self._project_id_by_name(project) if project_id == 0: sys.stderr.write( - "Note: No Project found matching %s, " - "ignoring filter\n" % project + f"Note: No Project found matching {project}, ignoring filter\n" ) else: filters['project_id'] = project_id @@ -296,7 +294,7 @@ def patch_list( person_ids = self._person_ids_by_name(submitter) if len(person_ids) == 0: sys.stderr.write( - "Note: Nobody found matching *%s*\n" % submitter + f"Note: Nobody found matching *{submitter}*\n" ) else: for person_id in person_ids: @@ -309,7 +307,7 @@ def patch_list( delegate_ids = self._person_ids_by_name(delegate) if len(delegate_ids) == 0: sys.stderr.write( - "Note: Nobody found matching *%s*\n" % delegate + f"Note: Nobody found matching *{delegate}*\n" ) else: for delegate_id in delegate_ids: @@ -324,7 +322,7 @@ def patch_get(self, patch_id): patch = self._client.patch_get(patch_id) if patch == {}: raise exceptions.APIError( - 'Unable to fetch patch %d; does it exist?' % patch_id + f'Unable to fetch patch {int(patch_id)}; does it exist?' ) return self._decode_patch(patch) @@ -341,7 +339,7 @@ def patch_get_mbox(self, patch_id): mbox = self._client.patch_get_mbox(patch_id) if len(mbox) == 0: raise exceptions.APIError( - 'Unable to fetch mbox for patch %d; does it exist?' % patch_id + f'Unable to fetch mbox for patch {int(patch_id)}; does it exist?' ) return mbox, patch['filename'] @@ -362,7 +360,7 @@ def patch_set( state_id = self._state_id_by_name(state) if state_id == 0: raise exceptions.APIError( - 'No State found matching %s*' % state + f'No State found matching {state}*' ) sys.exit(1) @@ -378,7 +376,7 @@ def patch_set( self._client.patch_set(patch_id, params) except xmlrpclib.Fault as f: raise exceptions.APIError( - 'Error updating patch: %s' % f.faultString + f'Error updating patch: {f.faultString}' ) # states @@ -424,7 +422,7 @@ def check_create( ) except xmlrpclib.Fault as f: raise exceptions.APIError( - 'Error creating check: %s' % f.faultString + f'Error creating check: {f.faultString}' ) diff --git a/pwclient/checks.py b/pwclient/checks.py index 19ade65..19558dd 100644 --- a/pwclient/checks.py +++ b/pwclient/checks.py @@ -23,7 +23,7 @@ def action_list(api, patch_id=None, user=None): def action_info(api, patch_id, check_id): check = api.check_get(patch_id, check_id) - s = "Information for check id %d" % (check_id) + s = f"Information for check id {int(check_id)}" print(s) print('-' * len(s)) for key, value in sorted(check.items()): @@ -46,14 +46,14 @@ def check_field(matchobj): for check in checks: print(format_field_re.sub(check_field, format_str)) else: - s = "Check information for patch id %d" % patch_id + s = f"Check information for patch id {int(patch_id)}" print(s) print('-' * len(s)) out = [] for check in checks: cout = [] for key, value in sorted(check.items()): - value = ' ' + str(value) if value else value + value = f" {str(value)}" if value else value cout.append("- %- 14s:%s" % (key, value)) out.append("\n".join(cout)) print("\n\n".join(out)) diff --git a/pwclient/patches.py b/pwclient/patches.py index 67e9fc9..6c89cbb 100644 --- a/pwclient/patches.py +++ b/pwclient/patches.py @@ -124,7 +124,7 @@ def action_info(api, patch_id): print(str(exc), file=sys.stderr) sys.exit(1) - s = "Information for patch id %d" % (patch_id) + s = f"Information for patch id {int(patch_id)}" print(s) print('-' * len(s)) for key, value in sorted(patch.items()): @@ -145,12 +145,12 @@ def action_get(api, patch_id): fname += '.patch' i = 0 while os.path.exists(fname): - fname = "%s.%d.patch" % (base_fname, i) + fname = f"{base_fname}.{int(i)}.patch" i += 1 with io.open(fname, 'x', encoding='utf-8') as f: f.write(mbox) - print('Saved patch to %s' % fname) + print(f'Saved patch to {fname}') def action_view(api, patch_ids): @@ -197,14 +197,14 @@ def action_apply(api, patch_id, apply_cmd=None): sys.exit(1) if apply_cmd is None: - print('Applying patch #%d to current directory' % patch_id) + print(f'Applying patch #{int(patch_id)} to current directory') apply_cmd = ['patch', '-p1'] else: print( - 'Applying patch #%d using "%s"' % (patch_id, ' '.join(apply_cmd)) + f'Applying patch #{int(patch_id)} using "{' '.join(apply_cmd)}"' ) - print('Description: %s' % patch['name']) + print(f"Description: {patch['name']}") try: mbox, _ = api.patch_get_mbox(patch_id) diff --git a/pwclient/shell.py b/pwclient/shell.py index fad63a7..8991adf 100644 --- a/pwclient/shell.py +++ b/pwclient/shell.py @@ -39,7 +39,7 @@ def main(argv=sys.argv[1:]): action = args.subcmd if not os.path.exists(CONFIG_FILE): - sys.stderr.write('Config file not found at %s.\n' % CONFIG_FILE) + sys.stderr.write(f'Config file not found at {CONFIG_FILE}.\n') sys.exit(1) # grab settings from config files @@ -52,8 +52,7 @@ def main(argv=sys.argv[1:]): if not config.has_section('options'): sys.stderr.write( - 'No options section in %s. Did you forget to uncomment it?\n' - % CONFIG_FILE + f'No options section in {CONFIG_FILE}. Did you forget to uncomment it?\n' ) sys.exit(1) @@ -67,19 +66,19 @@ def main(argv=sys.argv[1:]): configparser.NoOptionError, ): sys.stderr.write( - 'No default project configured in %s\n' % CONFIG_FILE + f'No default project configured in {CONFIG_FILE}\n' ) sys.exit(1) if not config.has_section(project_str): sys.stderr.write( - 'No section for project %s in %s\n' % (project_str, CONFIG_FILE) + f'No section for project {project_str} in {CONFIG_FILE}\n' ) sys.exit(1) if not config.has_option(project_str, 'url'): sys.stderr.write( - 'No URL for project %s in %s\n' % (project_str, CONFIG_FILE) + f'No URL for project {project_str} in {CONFIG_FILE}\n' ) sys.exit(1) @@ -111,9 +110,9 @@ def main(argv=sys.argv[1:]): or config.has_option(project_str, 'token') ): sys.stderr.write( - "The %s action requires authentication, but no " + f"The {action} action requires authentication, but no " "username/password or\n" - "token is configured\n" % action + "token is configured\n" ) sys.exit(1) else: @@ -122,9 +121,9 @@ def main(argv=sys.argv[1:]): and config.has_option(project_str, 'password') ): sys.stderr.write( - "The %s action requires authentication, but no " + f"The {action} action requires authentication, but no " "username or password\n" - "is configured\n" % action + "is configured\n" ) sys.exit(1) @@ -195,7 +194,7 @@ def main(argv=sys.argv[1:]): for patch_id in patch_ids: ret = patches.action_apply(api, patch_id) if ret: - sys.stderr.write("Apply failed with exit status %d\n" % ret) + sys.stderr.write(f"Apply failed with exit status {int(ret)}\n") sys.exit(1) elif action == 'git_am': @@ -237,7 +236,7 @@ def main(argv=sys.argv[1:]): for patch_id in patch_ids: ret = patches.action_apply(api, patch_id, cmd) if ret: - sys.stderr.write("'git am' failed with exit status %d\n" % ret) + sys.stderr.write(f"'git am' failed with exit status {int(ret)}\n") sys.exit(1) elif action == 'update': diff --git a/pwclient/utils.py b/pwclient/utils.py index 16da042..1955caa 100644 --- a/pwclient/utils.py +++ b/pwclient/utils.py @@ -11,7 +11,7 @@ def migrate_old_config_file(config_file, config): """Convert a config file to the Patchwork 1.0 format.""" - sys.stderr.write('%s is in the old format. Migrating it...' % config_file) + sys.stderr.write(f'{config_file} is in the old format. Migrating it...') old_project = config.get('base', 'project') @@ -27,7 +27,7 @@ def migrate_old_config_file(config_file, config): if config.has_option('auth', 'password'): new_config.set(old_project, 'password', config.get('auth', 'password')) - old_config_file = config_file + '.orig' + old_config_file = f"{config_file}.orig" shutil.copy2(config_file, old_config_file) with open(config_file, 'w') as fd: @@ -35,7 +35,7 @@ def migrate_old_config_file(config_file, config): sys.stderr.write(' Done.\n') sys.stderr.write( - 'Your old %s was saved to %s\n' % (config_file, old_config_file) + f'Your old {config_file} was saved to {old_config_file}\n' ) sys.stderr.write('and was converted to the new format. You may want to\n') sys.stderr.write('inspect it before continuing.\n') diff --git a/pwclient/xmlrpc.py b/pwclient/xmlrpc.py index c443aa0..f2c1fcc 100644 --- a/pwclient/xmlrpc.py +++ b/pwclient/xmlrpc.py @@ -24,21 +24,21 @@ def __init__(self, url): self.https = self.proxy.startswith('https') def set_credentials(self, username=None, password=None): - self.credentials = '%s:%s' % (username, password) + self.credentials = f'{username}:{password}' def make_connection(self, host): self.host = host if self.proxy: host = self.proxy.split('://', 1)[-1].rstrip('/') if self.credentials: - host = '@'.join([self.credentials, host]) + host = f"{self.credentials}@{host}" if self.https: return xmlrpclib.SafeTransport.make_connection(self, host) else: return xmlrpclib.Transport.make_connection(self, host) def send_request(self, host, handler, request_body, debug): - handler = '%s://%s%s' % (self.scheme, host, handler) + handler = f'{self.scheme}://{host}{handler}' return xmlrpclib.Transport.send_request( self, host, From 263bee33cf4f3f88c573ce66df7888178cf5a779 Mon Sep 17 00:00:00 2001 From: Sean Khan Date: Thu, 15 May 2025 13:05:13 -0400 Subject: [PATCH 2/2] feature: add date column when displaying patches Add support for displaying the original submission date of patches because it is useful to know when the patch was submitted. Signed-off-by: Sean Khan --- pwclient/patches.py | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/pwclient/patches.py b/pwclient/patches.py index 6c89cbb..7fffabf 100644 --- a/pwclient/patches.py +++ b/pwclient/patches.py @@ -48,11 +48,32 @@ def patch_field(matchobj): for patch in patches: print(format_field_re.sub(patch_field, format_str)) else: - print("%-7s %-12s %s" % ("ID", "State", "Name")) - print("%-7s %-12s %s" % ("--", "-----", "----")) + # ID and State need to be the same width as the longest ID and State + width_id, width_state, width_name = 2, 5, 30 for patch in patches: + width_id = max(width_id, len(str(patch['id']))) + width_state = max(width_state, len(patch['state'])) + width_name = max(width_name, len(patch['name'])) + print( + f"{'Date':<10} " + f"{'ID':<{width_id}} " + f"{'State':<{width_state}} " + f"{'Name':<{width_name}}" + ) + print( + f"{'-'*10} " + f"{'-'*width_id} " + f"{'-'*width_state} " + f"{'-'*width_name}" + ) + + for patch in patches: + date_only = patch['date'].split(' ')[0] print( - "%-7d %-12s %s" % (patch['id'], patch['state'], patch['name']) + f"{date_only:<10} " + f"{patch['id']:<{width_id}} " + f"{patch['state']:<{width_state}} " + f"{patch['name']:<{width_name}} " )