Skip to content

refactor: use shopen instead of manual launch_editor#4

Merged
snomiao merged 1 commit intomainfrom
refactor/shopen-editor-clean
Feb 17, 2026
Merged

refactor: use shopen instead of manual launch_editor#4
snomiao merged 1 commit intomainfrom
refactor/shopen-editor-clean

Conversation

@snomiao
Copy link
Copy Markdown
Owner

@snomiao snomiao commented Feb 17, 2026

Ported from insanum/gcalcli#767 by @dbarnett. Conflict-resolved against this fork's main.

Summary

  • Replaces hand-rolled launch_editor (which tried editor/$EDITOR/xdg-open/open in sequence) with the shopen library
  • Removes launch_editor from utils.py, along with now-unused os and subprocess imports
  • Adds shopen >= 0.2.2 to pyproject.toml dependencies
  • Net: -23/+4 lines

Why

The shopen library handles cross-platform file opening/editing more robustly than the manual fallback chain.

🤖 Generated with Claude Code

…anum#767)

Replace the hand-rolled launch_editor utility (which tried editor/EDITOR/
xdg-open/open in sequence) with the shopen library. Removes subprocess
and os imports from utils.py. Conflict-resolved against main.

Co-Authored-By: David Barnett <noreply@github.com>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings February 17, 2026 11:14
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors the CLI’s “edit config” workflow to rely on the shopen library for cross-platform “open/edit” behavior, replacing the project’s hand-rolled editor/launcher logic.

Changes:

  • Add shopen as a runtime dependency.
  • Remove launch_editor (and now-unused imports) from gcalcli/utils.py.
  • Update gcalcli config edit to call shopen.open(..., 'edit').

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
pyproject.toml Adds the shopen dependency required by the new config-edit implementation.
gcalcli/utils.py Removes the legacy launch_editor helper and associated unused imports.
gcalcli/cli.py Switches the config edit subcommand to use shopen for launching an editor.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 349 to +352
config_filepath.parent.mkdir(parents=True, exist_ok=True)
with open(config_filepath, 'w') as f:
f.write(EMPTY_CONFIG_TOML)
utils.launch_editor(config_filepath)
shopen.open(config_filepath, 'edit')
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

The new config edit path now depends on calling shopen.open(...), but there doesn’t appear to be any automated test covering this behavior (the existing CLI bats tests cover other commands but not config edit). Please add a test that exercises gcalcli config edit while stubbing/monkeypatching shopen.open so the test can assert it’s invoked with edit without launching a real editor, and that the config file is created when missing.

Copilot uses AI. Check for mistakes.
"platformdirs",
"pydantic",
"python-dateutil",
"shopen >= 0.2.2",
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

Dependency spec formatting is inconsistent with the rest of this file: other requirements are written without spaces around the version operator (e.g., google-api-python-client>=1.4). Consider changing "shopen >= 0.2.2" to "shopen>=0.2.2" for consistency and to avoid potential issues with tools that do strict parsing.

Suggested change
"shopen >= 0.2.2",
"shopen>=0.2.2",

Copilot uses AI. Check for mistakes.
@snomiao snomiao merged commit caa7584 into main Feb 17, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants