Skip to content

Commit 01d1040

Browse files
committed
style: Fix code style violations (Phase 1)
- Fix line length issues (E501) in CLI modules - Replace open() with Path.open() (PTH123) - Remove redundant exception objects in logging.exception() calls (TRY401) - Break long strings across multiple lines for better readability - All ruff and mypy checks now pass for the new CLI commands
1 parent bd34982 commit 01d1040

File tree

3 files changed

+106
-68
lines changed

3 files changed

+106
-68
lines changed

src/vcspull/cli/__init__.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,8 @@ def create_parser(
108108
"add-from-fs",
109109
help="scan filesystem for git repositories and add them to the configuration",
110110
formatter_class=argparse.RawDescriptionHelpFormatter,
111-
description="Scan a directory for git repositories and add them to the vcspull configuration file.",
111+
description="Scan a directory for git repositories and add them to the "
112+
"vcspull configuration file.",
112113
)
113114
create_add_from_fs_subparser(add_from_fs_parser)
114115

@@ -143,7 +144,11 @@ def cli(_args: list[str] | None = None) -> None:
143144
"repo_patterns": args.repo_patterns
144145
if hasattr(args, "repo_patterns")
145146
else None,
146-
"config": pathlib.Path(args.config) if hasattr(args, "config") and args.config else None,
147+
"config": (
148+
pathlib.Path(args.config)
149+
if hasattr(args, "config") and args.config
150+
else None
151+
),
147152
"exit_on_error": args.exit_on_error
148153
if hasattr(args, "exit_on_error")
149154
else False,

src/vcspull/cli/add.py

Lines changed: 74 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -30,17 +30,20 @@ def create_add_subparser(parser: argparse.ArgumentParser) -> None:
3030
)
3131
parser.add_argument(
3232
"repo_name_or_url",
33-
help="Name of the repository or its URL. If only name, URL may be inferred or prompted.",
33+
help="Name of the repository or its URL. "
34+
"If only name, URL may be inferred or prompted.",
3435
)
3536
parser.add_argument(
3637
"target_path",
3738
nargs="?",
38-
help="Optional local path for the repository (can be relative or absolute). Overrides default naming.",
39+
help="Optional local path for the repository (can be relative or absolute). "
40+
"Overrides default naming.",
3941
)
4042
parser.add_argument(
4143
"--base-dir-key",
42-
help="Specify the top-level directory key from vcspull config (e.g., '~/study/python/') under which to add this repo."
43-
" If not given, vcspull will try to infer or use the current working directory.",
44+
help="Specify the top-level directory key from vcspull config "
45+
"(e.g., '~/study/python/') under which to add this repo. "
46+
"If not given, vcspull will try to infer or use the current working directory.",
4447
)
4548

4649

@@ -59,13 +62,16 @@ def add_repo(
5962
# Behavior of find_home_config_files might need adjustment if it returns a list
6063
home_configs = find_home_config_files(filetype=["yaml"])
6164
if not home_configs:
62-
# If no global --config and no home config, default to creating .vcspull.yaml in CWD
65+
# If no global --config and no home config, default to
66+
# creating .vcspull.yaml in CWD
6367
config_file_path = pathlib.Path.cwd() / ".vcspull.yaml"
6468
log.info(
65-
f"No config specified and no default found, will create at {config_file_path}",
69+
f"No config specified and no default found, will create at "
70+
f"{config_file_path}",
6671
)
6772
elif len(home_configs) > 1:
68-
# This case should ideally be handled by the main CLI arg parsing for --config
73+
# This case should ideally be handled by the main CLI arg parsing
74+
# for --config
6975
log.error(
7076
"Multiple home_config files found, please specify one with -c/--config",
7177
)
@@ -76,15 +82,16 @@ def add_repo(
7682
raw_config: dict[str, t.Any] = {}
7783
if config_file_path.exists() and config_file_path.is_file():
7884
try:
79-
with open(config_file_path, encoding="utf-8") as f:
85+
with config_file_path.open(encoding="utf-8") as f:
8086
raw_config = yaml.safe_load(f) or {}
8187
if not isinstance(raw_config, dict):
8288
log.error(
83-
f"Config file {config_file_path} is not a valid YAML dictionary. Aborting.",
89+
f"Config file {config_file_path} is not a valid YAML dictionary. "
90+
"Aborting.",
8491
)
8592
return
86-
except Exception as e:
87-
log.exception(f"Error loading YAML from {config_file_path}: {e}. Aborting.")
93+
except Exception:
94+
log.exception(f"Error loading YAML from {config_file_path}. Aborting.")
8895
return
8996
else:
9097
log.info(
@@ -94,8 +101,8 @@ def add_repo(
94101
repo_name: str
95102
repo_url: str
96103

97-
# Simplistic parsing of repo_name_or_url, assuming it's a URL if it contains '://' or 'git@'
98-
# A more robust solution would involve libvcs or regex
104+
# Simplistic parsing of repo_name_or_url, assuming it's a URL if it
105+
# contains '://' or 'git@'. A more robust solution would involve libvcs or regex
99106
if "://" in repo_name_or_url or repo_name_or_url.startswith(
100107
("git@", "git+", "hg+", "svn+"),
101108
):
@@ -108,34 +115,42 @@ def add_repo(
108115
).name # if target_path is specified, use its name for repo_name
109116
else:
110117
repo_name = repo_name_or_url
111-
# URL is not provided, for now, we can make it an error or set a placeholder
112-
# In future, could try to infer from a local git repo if target_path_str points to one
118+
# URL is not provided, for now, we can make it an error or set a
119+
# placeholder. In future, could try to infer from a local git repo if
120+
# target_path_str points to one
113121
log.error(
114-
"Repository URL must be provided if repo_name_or_url is not a URL. Functionality to infer URL is not yet implemented.",
122+
"Repository URL must be provided if repo_name_or_url is not a URL. "
123+
"Functionality to infer URL is not yet implemented.",
115124
)
116-
# For now, let's assume a placeholder or require URL to be part of repo_name_or_url
117-
# This part needs a decision: either make URL mandatory with name, or implement inference
118-
# Let's require the user to provide a URL for now by just using repo_name_or_url as URL if it seems like one
119-
# This means the first arg must be a URL if only one arg is given and it's not just a name.
120-
# To simplify, let's assume if not detected as URL, then it's just a name and we need target_path to be a URL
121-
# Or, the user *must* provide two arguments: name then url. The current subparser takes one `repo_name_or_url` and optional `target_path`.
122-
# This argument parsing needs refinement in `create_add_subparser` to accept name and url separately.
123-
# For now, this will likely fail if a plain name is given without a clear URL source.
124-
# Let's adjust the expectation: repo_name_or_url IS THE URL, and repo_name is derived or taken from target_path.
125-
# This means first arg is always URL-like or repo name. Second is target_path (which becomes name)
126-
127-
# Re-evaluating argument strategy based on user request for `vcspull add my-repo <url>`:
128-
# `repo_name_or_url` will be `my-repo`
129-
# `target_path` will be `<url>`
130-
# This means `create_add_subparser` needs adjustment.
131-
# For now, let's proceed with the current subparser and assume `repo_name_or_url` is the name, and `target_path` (if provided) is the URL for this specific case.
132-
# This is getting complicated. Let's simplify the add command for now:
133-
# `vcspull add <NAME> <URL>`
125+
# For now, let's assume a placeholder or require URL to be part of
126+
# repo_name_or_url. This part needs a decision: either make URL mandatory
127+
# with name, or implement inference. Let's require the user to provide a
128+
# URL for now by just using repo_name_or_url as URL if it seems like one.
129+
# This means the first arg must be a URL if only one arg is given and it's
130+
# not just a name. To simplify, let's assume if not detected as URL, then
131+
# it's just a name and we need target_path to be a URL. Or, the user
132+
# *must* provide two arguments: name then url. The current subparser takes
133+
# one `repo_name_or_url` and optional `target_path`. This argument parsing
134+
# needs refinement in `create_add_subparser` to accept name and url
135+
# separately. For now, this will likely fail if a plain name is given
136+
# without a clear URL source. Let's adjust the expectation: repo_name_or_url
137+
# IS THE URL, and repo_name is derived or taken from target_path. This means
138+
# first arg is always URL-like or repo name. Second is target_path (which
139+
# becomes name)
140+
141+
# Re-evaluating argument strategy based on user request for
142+
# `vcspull add my-repo <url>`: `repo_name_or_url` will be `my-repo`
143+
# `target_path` will be `<url>`. This means `create_add_subparser` needs
144+
# adjustment. For now, let's proceed with the current subparser and assume
145+
# `repo_name_or_url` is the name, and `target_path` (if provided) is the
146+
# URL for this specific case. This is getting complicated. Let's simplify
147+
# the add command for now: `vcspull add <NAME> <URL>`
134148
# The current `repo_name_or_url` and `target_path` can be repurposed.
135149

136150
# Let's rename parser args for clarity: repo_name, repo_url
137151
# This requires changing create_add_subparser and the cli call in __init__.py
138-
# Given the current structure, repo_name_or_url = actual name, target_path = actual_url
152+
# Given the current structure, repo_name_or_url = actual name,
153+
# target_path = actual_url
139154
if target_path_str is None:
140155
log.error("If first argument is a name, second argument (URL) is required.")
141156
return
@@ -151,14 +166,17 @@ def add_repo(
151166
elif target_path_str and not (
152167
"://" in target_path_str or target_path_str.startswith("git@")
153168
): # if target_path is a path, not url
154-
# Infer from target_path_str if it's a local path
155-
# This assumes target_path_str is the actual clone path if provided and is not the URL itself
156-
# This logic is getting tangled due to the argument overloading. Let's stick to the idea that repo_name is derived or is first arg, url is second or inferred
157-
# For simplicity, let's assume if base_dir_key is not given, we use a default or CWD based key
158-
# This needs to be more robust. Example: if repo is to be cloned to /home/user/study/python/myrepo
169+
# Infer from target_path_str if it's a local path. This assumes
170+
# target_path_str is the actual clone path if provided and is not the URL
171+
# itself. This logic is getting tangled due to the argument overloading.
172+
# Let's stick to the idea that repo_name is derived or is first arg, url
173+
# is second or inferred. For simplicity, let's assume if base_dir_key is
174+
# not given, we use a default or CWD based key. This needs to be more
175+
# robust. Example: if repo is to be cloned to /home/user/study/python/myrepo
159176
# and vcspull.yaml has '~/study/python/': ..., then this is the key.
160177
# We can use os.path.commonpath or iterate through existing keys.
161-
# For now, default to current working directory's representation or a new top-level key if no match.
178+
# For now, default to current working directory's representation or a new
179+
# top-level key if no match.
162180
cwd_path = pathlib.Path.cwd()
163181
resolved_target_dir_parent = (
164182
pathlib.Path(target_path_str).parent.resolve()
@@ -174,7 +192,8 @@ def add_repo(
174192
resolved_target_dir_parent == expanded_key_path
175193
or resolved_target_dir_parent.is_relative_to(expanded_key_path)
176194
):
177-
# Check if resolved_target_dir_parent is a subdirectory or same as an existing key path
195+
# Check if resolved_target_dir_parent is a subdirectory or same as
196+
# an existing key path
178197
try:
179198
if resolved_target_dir_parent.relative_to(expanded_key_path):
180199
found_key = key_path_str
@@ -197,19 +216,22 @@ def add_repo(
197216
except ValueError:
198217
actual_base_dir_key = str(resolved_target_dir_parent) + "/"
199218
else:
200-
# Default base directory key if not specified and target_path is not a local path
219+
# Default base directory key if not specified and target_path is not a
220+
# local path
201221
# This could be "." or a user-configurable default
202222
actual_base_dir_key = "./" # Default to relative current directory key
203223

204-
if not actual_base_dir_key.endswith("/"): # Ensure trailing slash for consistency
224+
# Ensure trailing slash for consistency
225+
if not actual_base_dir_key.endswith("/"):
205226
actual_base_dir_key += "/"
206227

207228
# Ensure the base directory key exists in the config
208229
if actual_base_dir_key not in raw_config:
209230
raw_config[actual_base_dir_key] = {}
210231
elif not isinstance(raw_config[actual_base_dir_key], dict):
211232
log.error(
212-
f"Configuration section '{actual_base_dir_key}' is not a dictionary. Aborting.",
233+
f"Configuration section '{actual_base_dir_key}' is not a dictionary. "
234+
"Aborting.",
213235
)
214236
return
215237

@@ -224,14 +246,15 @@ def add_repo(
224246

225247
# Add the repository
226248
# Simplest form: {repo_name: repo_url}
227-
# More complex form (if remotes, etc., were supported): {repo_name: {"url": repo_url, ...}}
249+
# More complex form (if remotes, etc., were supported):
250+
# {repo_name: {"url": repo_url, ...}}
228251
raw_config[actual_base_dir_key][repo_name] = repo_url
229252

230253
try:
231254
# save_config_yaml(config_file_path, raw_config) # Original call to local/helper
232255
# The main application code should use a robust save mechanism.
233256
# For now, directly writing, similar to the old save_config_yaml.
234-
with open(config_file_path, "w", encoding="utf-8") as f:
257+
with config_file_path.open("w", encoding="utf-8") as f:
235258
yaml.dump(
236259
raw_config,
237260
f,
@@ -240,7 +263,8 @@ def add_repo(
240263
default_flow_style=False,
241264
)
242265
log.info(
243-
f"Successfully added '{repo_name}' ({repo_url}) to {config_file_path} under '{actual_base_dir_key}'.",
266+
f"Successfully added '{repo_name}' ({repo_url}) to {config_file_path} "
267+
f"under '{actual_base_dir_key}'.",
244268
)
245-
except Exception as e:
246-
log.exception(f"Error saving config to {config_file_path}: {e}")
269+
except Exception:
270+
log.exception(f"Error saving config to {config_file_path}")

src/vcspull/cli/add_from_fs.py

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ def get_git_origin_url(repo_path: pathlib.Path) -> str | None:
4040

4141
def save_config_yaml(config_file_path: pathlib.Path, data: dict[t.Any, t.Any]) -> None:
4242
"""Save a dictionary to a YAML file."""
43-
with open(config_file_path, "w", encoding="utf-8") as f:
43+
with config_file_path.open("w", encoding="utf-8") as f:
4444
yaml.dump(data, f, sort_keys=False, indent=2, default_flow_style=False)
4545

4646

@@ -67,8 +67,10 @@ def create_add_from_fs_subparser(parser: argparse.ArgumentParser) -> None:
6767
)
6868
parser.add_argument(
6969
"--base-dir-key",
70-
help="Specify the top-level directory key from vcspull config (e.g., '~/study/python/') under which to add these repos."
71-
" If not given, the normalized absolute path of scan_dir will be used as the key.",
70+
help="Specify the top-level directory key from vcspull config "
71+
"(e.g., '~/study/python/') under which to add these repos. "
72+
"If not given, the normalized absolute path of scan_dir will be used as "
73+
"the key.",
7274
)
7375
parser.add_argument(
7476
"--yes",
@@ -96,7 +98,8 @@ def add_from_filesystem(
9698
if not home_configs:
9799
config_file_path = pathlib.Path.cwd() / ".vcspull.yaml"
98100
log.info(
99-
f"No config specified and no default home config, will use/create {config_file_path}",
101+
f"No config specified and no default home config, will use/create "
102+
f"{config_file_path}",
100103
)
101104
elif len(home_configs) > 1:
102105
log.error(
@@ -109,15 +112,16 @@ def add_from_filesystem(
109112
raw_config: dict[str, t.Any] = {}
110113
if config_file_path.exists() and config_file_path.is_file():
111114
try:
112-
with open(config_file_path, encoding="utf-8") as f:
115+
with config_file_path.open(encoding="utf-8") as f:
113116
raw_config = yaml.safe_load(f) or {}
114117
if not isinstance(raw_config, dict):
115118
log.error(
116-
f"Config file {config_file_path} is not a valid YAML dictionary. Aborting.",
119+
f"Config file {config_file_path} is not a valid YAML dictionary. "
120+
"Aborting.",
117121
)
118122
return
119123
except Exception as e:
120-
log.exception(f"Error loading YAML from {config_file_path}: {e}. Aborting.")
124+
log.exception(f"Error loading YAML from {config_file_path}. Aborting.")
121125
return
122126
else:
123127
log.info(
@@ -136,7 +140,8 @@ def add_from_filesystem(
136140

137141
if not repo_url:
138142
log.warning(
139-
f"Could not determine remote URL for git repository at {repo_path}. Skipping.",
143+
f"Could not determine remote URL for git repository at "
144+
f"{repo_path}. Skipping.",
140145
)
141146
continue
142147

@@ -148,10 +153,11 @@ def add_from_filesystem(
148153
else base_dir_key_arg + "/"
149154
)
150155
else:
151-
# Try to make it relative to home if possible, otherwise absolute path of scan_dir parent
152-
# This should be the parent of the repo_path itself, relative to scan_dir or absolute.
153-
# Or simply use scan_dir as the key.
154-
# Let's use the provided scan_dir (normalized) as the default key if not given.
156+
# Try to make it relative to home if possible, otherwise absolute path
157+
# of scan_dir parent. This should be the parent of the repo_path itself,
158+
# relative to scan_dir or absolute. Or simply use scan_dir as the key.
159+
# Let's use the provided scan_dir (normalized) as the default key if
160+
# not given.
155161
try:
156162
determined_base_key = (
157163
"~/" + str(scan_dir.relative_to(pathlib.Path.home())) + "/"
@@ -182,7 +188,8 @@ def add_from_filesystem(
182188

183189
if not repos_to_add:
184190
log.info(
185-
"All found repositories already exist in the configuration. Nothing to do.",
191+
"All found repositories already exist in the configuration. "
192+
"Nothing to do.",
186193
)
187194
return
188195

@@ -199,7 +206,8 @@ def add_from_filesystem(
199206
repos_to_add.append((name, url, key))
200207
if not repos_to_add:
201208
log.info(
202-
"All found repositories already exist in the configuration or were skipped. Nothing to do.",
209+
"All found repositories already exist in the configuration or were "
210+
"skipped. Nothing to do.",
203211
)
204212
return
205213

@@ -209,7 +217,8 @@ def add_from_filesystem(
209217
raw_config[determined_base_key] = {}
210218
elif not isinstance(raw_config[determined_base_key], dict):
211219
log.warning(
212-
f"Section '{determined_base_key}' in config is not a dictionary. Skipping repo {repo_name}.",
220+
f"Section '{determined_base_key}' in config is not a dictionary. "
221+
f"Skipping repo {repo_name}.",
213222
)
214223
continue
215224

@@ -226,6 +235,6 @@ def add_from_filesystem(
226235
save_config_yaml(config_file_path, raw_config)
227236
log.info(f"Successfully updated {config_file_path}.")
228237
except Exception as e:
229-
log.exception(f"Error saving config to {config_file_path}: {e}")
238+
log.exception(f"Error saving config to {config_file_path}")
230239
else:
231240
log.info("No changes made to the configuration.")

0 commit comments

Comments
 (0)