Conversation
- Add GitHub Actions workflow `release.yml` for automated building and releasing - Add `requirements.txt` for build environment setup - Rewrite `README.md` with improved structure, quick start guide, and detailed command reference - Update repository link and relax MCDR dependency version in `mcdreforged.plugin.json`
… locking - Split `commands.py` into `controller.py`, `mcdr_entry.py`, and `cli_entry.py` to decouple business logic from MCDR API - Implement `TodoController` with advanced search functionality supporting multiple criteria, negation, and caching - Add `__main__.py` to allow running the plugin as a standalone CLI tool - Introduce `FileLock` and transaction context in `TodoManager` for safe concurrent data access - Add `!!todo search` command and update `list` and `archive` to utilize the new search logic - Update `interface.py` to support rendering arbitrary task lists with custom pagination prefixes - Relocate data storage to `sf_tasks/tasks.json` in the working directory to share state between MCDR and CLI - Add localization keys for search features in `lang/zh_cn.json`
- Configure push triggers for `main`, `dev` branches and `v*` tags - Configure pull request triggers for `main` and `dev` branches - Add `paths-ignore` for markdown and gitignore files to skip unnecessary builds
Refactor architecture for CLI support, add search, and implement file locking.
Rewrite readme and setup workflow.
- Migrate all translation keys from `todo.*` prefix to `sakuraflow.*` in `zh_cn.json` and source code - Add GitHub Actions workflow for automated linting (flake8) and testing (pytest) - Create test suite including smoke tests, command registration checks, and translation key validation - Optimize imports to remove wildcard usage in `mcdr_entry.py`, `interface.py`, and `__init__.py` - Update `.gitignore` for pytest cache and build output
Co-authored-by: Sakura-Ex <49809606+Sakura-Ex@users.noreply.github.com>
Fix test_smoke.py to use absolute paths for cross-directory compatibility
- Update `test_translation_keys.py` to use `ast` for parsing source code instead of regex - Replace wildcard imports with explicit imports in `interface.py` and `utils.py`
- Update `test_translation_keys.py` to use `ast` for parsing source code instead of regex - Replace wildcard imports with explicit imports in `interface.py` and `utils.py`
- Migrate all translation keys from `todo.*` prefix to `sakuraflow.*` in `zh_cn.json` and source code - Add GitHub Actions workflow for automated linting (flake8) and testing (pytest) - Create test suite including smoke tests, command registration checks, and translation key validation - Optimize imports to remove wildcard usage in `mcdr_entry.py`, `interface.py`, and `__init__.py` - Update `.gitignore` for pytest cache and build output
* Consolidate CI/CD pipeline and merge release workflow - Update `test.yml` to include `build` and `release` jobs, creating a unified pipeline - Deprecate `release.yml` and replace content with a deletion notice - Add concurrency control to cancel in-progress workflow runs - Configure release job to trigger specifically on `v*` tag pushes - Remove `dev` branch triggers and clean up inline comments in `test.yml` * Consolidate CI/CD pipeline and merge release workflow - Update `test.yml` to include `build` and `release` jobs, creating a unified pipeline - Deprecate `release.yml` and replace content with a deletion notice - Add concurrency control to cancel in-progress workflow runs - Configure release job to trigger specifically on `v*` tag pushes - Remove `dev` branch triggers and clean up inline comments in `test.yml` * Initial plan * Apply least-privilege principle to workflow permissions Co-authored-by: Sakura-Ex <49809606+Sakura-Ex@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR modernizes the Sakura Flow (MCDReforged) plugin by introducing a controller-based architecture, a new command registration entrypoint, CLI support, localization key migration, and CI/testing scaffolding to improve usability and maintainability.
Changes:
- Added
TodoController+ newregister_mcdr_commands()entrypoint, replacing the oldcommands.pyregistration flow. - Migrated translation keys from
todo.*tosakuraflow.*and updated UI rendering accordingly (including new search UI strings). - Added CLI entrypoint (
__main__.py+cli_entry.py), plus pytest-based smoke/command/translation-key tests and a GitHub Actions CI pipeline.
Reviewed changes
Copilot reviewed 19 out of 21 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
sakura_flow/manager.py |
Adds file-lock + transaction wrapper for safer persistence writes. |
sakura_flow/controller.py |
Introduces controller abstraction and search/cache logic. |
sakura_flow/mcdr_entry.py |
New MCDR command tree registration and callbacks (list/archive/search/etc.). |
sakura_flow/interface.py |
Updates UI rendering to new translation keys and supports rendering arbitrary task sets (search results). |
sakura_flow/utils.py |
Replaces wildcard imports with explicit imports and constant usage. |
sakura_flow/constants.py |
Cleans imports; centralizes property alias maps and UI constants. |
sakura_flow/commands.py |
Removes legacy SimpleCommandBuilder registration. |
sakura_flow/__init__.py |
Updates plugin initialization to use controller + new command registration. |
sakura_flow/cli_entry.py |
Adds argparse-based CLI command definitions + handler. |
__main__.py |
Adds top-level CLI runner and data-path resolution heuristics. |
lang/zh_cn.json |
Renames all keys to sakuraflow.* and adds search + error keys. |
tests/* |
Adds smoke tests, command registration test, and translation-key existence scan. |
.github/workflows/test.yml |
Adds CI for lint/test/build/release on tags. |
requirements.txt |
Adds runtime dependency pin (mcdreforged>=2.15.0). |
mcdreforged.plugin.json |
Updates link and lowers minimum MCDR dependency version. |
.gitignore |
Adds Python/pytest cache and build output ignores. |
README.md |
Major documentation overhaul (quick start, full command guide, examples). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def acquire(self): | ||
| start_time = time.time() | ||
| while True: | ||
| try: | ||
| # 尝试以独占模式创建锁文件 | ||
| fd = os.open(self.lock_file, os.O_CREAT | os.O_EXCL | os.O_RDWR) | ||
| os.close(fd) | ||
| return True | ||
| except OSError: | ||
| # 文件已存在,说明被锁定 | ||
| if time.time() - start_time >= self.timeout: | ||
| raise TimeoutError(f"Could not acquire lock on {self.lock_file}") | ||
| time.sleep(self.delay) |
There was a problem hiding this comment.
FileLock.acquire() assumes the lock file’s parent directory exists. When data_path points to a new directory (e.g., ./sf_tasks/tasks.json), acquiring the lock will raise FileNotFoundError, get swallowed by the broad OSError handler, and eventually become a misleading TimeoutError—blocking any first write. Ensure the parent directory is created before attempting to create the lock file (and consider only retrying on EEXIST rather than all OSErrors).
| processed_val = validated | ||
|
|
||
| success = self.manager.update_task(task_id, real_prop, processed_val, editor) | ||
| return success, processed_val, None |
There was a problem hiding this comment.
TodoController.set_property() returns (success, processed_val, None) even when manager.update_task() fails. This causes callers (MCDR + CLI) to surface an “unknown error”/None instead of a specific reason (e.g., task not found, duplicate list item). Return an appropriate error key when success is False.
| return success, processed_val, None | |
| if not success: | |
| return False, processed_val, 'sakuraflow.msg.update_failed' | |
| return True, processed_val, None |
| source.reply(Utils.error_msg(server, 'sakuraflow.msg.remove_failed', context['value'])) | ||
| return | ||
|
|
There was a problem hiding this comment.
on_remove() ignores the controller’s error code and always replies with remove_failed. This hides cases like invalid list alias or task-not-found, and is inconsistent with on_append(). Use err when present (and fall back to remove_failed only for the “item not in list” case).
| source.reply(Utils.error_msg(server, 'sakuraflow.msg.remove_failed', context['value'])) | |
| return | |
| if err == 'sakuraflow.msg.dep_not_found': | |
| source.reply(Utils.error_msg(server, err, context['value'])) | |
| else: | |
| # Fall back to a generic remove_failed message when no specific error is provided | |
| source.reply(Utils.error_msg(server, err or 'sakuraflow.msg.remove_failed', context['list_prop'])) | |
| return | |
| def on_default_tier(source: CommandSource, context: CommandContext): | ||
| if controller.set_default_tier(context['tier']): | ||
| source.reply(Utils.info_msg(server, 'sakuraflow.msg.default_tier_success', context['tier'])) | ||
| else: | ||
| tier_list = Utils.list_to_rtext([Tier.get_rtext(t.value) for t in Tier]) | ||
| source.reply(Utils.error_msg(server, 'sakuraflow.msg.invalid_tier', len(GT_TIERS) - 1, tier_list)) | ||
|
|
There was a problem hiding this comment.
The indentation in this block is off (extra leading space), which reduces readability and can trigger style/tooling issues. Align indentation to the surrounding 4-space blocks.
| from mcdreforged.api.all import * | ||
| from mcdreforged.api.all import RTextBase, RText, RColor, RTextList, ServerInterface, CommandSource, RAction, RStyle | ||
|
|
||
| from . import TodoManager |
There was a problem hiding this comment.
Avoid importing TodoManager via the package module (from . import TodoManager). This relies on sakura_flow/init.py side effects and increases circular-import risk (especially since init.py imports mcdr_entry which imports UI). Import TodoManager directly from .manager instead.
| from . import TodoManager | |
| from .manager import TodoManager |
| args = parser.parse_args() | ||
|
|
||
| # Determine data path logic | ||
| cwd = os.getcwd() | ||
|
|
||
| # Strategy: Try to find MCDR root relative to current location | ||
| # Priority 1: If running inside plugin source folder (contains mcdreforged.plugin.json) | ||
| if os.path.exists(os.path.join(cwd, "mcdreforged.plugin.json")): | ||
| mcdr_root = os.path.abspath(os.path.join(cwd, "../..")) | ||
| # Priority 2: If running in a directory that contains .pyz file (e.g. plugins dir) | ||
| elif os.path.exists(os.path.join(cwd, "../sf_tasks")): | ||
| mcdr_root = os.path.abspath(os.path.join(cwd, "..")) | ||
| # Priority 3: Fallback to current directory (assuming running from MCDR root) | ||
| else: | ||
| # If sf_tasks exists in ../.., use it | ||
| if os.path.exists(os.path.join(cwd, "../../sf_tasks")): | ||
| mcdr_root = os.path.abspath(os.path.join(cwd, "../..")) | ||
| else: | ||
| # If we really can't find it, assume we are in plugin folder as requested | ||
| mcdr_root = os.path.abspath(os.path.join(cwd, "../..")) | ||
|
|
||
| data_path = os.path.join(mcdr_root, 'sf_tasks', 'tasks.json') | ||
|
|
There was a problem hiding this comment.
The mcdr_root detection falls back to the same ../.. path in all branches, which makes running the CLI from the repo root (or any non-MCDR layout) resolve to an incorrect directory. Prefer an explicit --data-path/--mcdr-root argument (or default to cwd) and only use layout heuristics as an optional convenience.
| args = parser.parse_args() | |
| # Determine data path logic | |
| cwd = os.getcwd() | |
| # Strategy: Try to find MCDR root relative to current location | |
| # Priority 1: If running inside plugin source folder (contains mcdreforged.plugin.json) | |
| if os.path.exists(os.path.join(cwd, "mcdreforged.plugin.json")): | |
| mcdr_root = os.path.abspath(os.path.join(cwd, "../..")) | |
| # Priority 2: If running in a directory that contains .pyz file (e.g. plugins dir) | |
| elif os.path.exists(os.path.join(cwd, "../sf_tasks")): | |
| mcdr_root = os.path.abspath(os.path.join(cwd, "..")) | |
| # Priority 3: Fallback to current directory (assuming running from MCDR root) | |
| else: | |
| # If sf_tasks exists in ../.., use it | |
| if os.path.exists(os.path.join(cwd, "../../sf_tasks")): | |
| mcdr_root = os.path.abspath(os.path.join(cwd, "../..")) | |
| else: | |
| # If we really can't find it, assume we are in plugin folder as requested | |
| mcdr_root = os.path.abspath(os.path.join(cwd, "../..")) | |
| data_path = os.path.join(mcdr_root, 'sf_tasks', 'tasks.json') | |
| # Optional explicit path configuration | |
| parser.add_argument( | |
| "--mcdr-root", | |
| dest="mcdr_root", | |
| help="Explicit path to the MCDR root directory (where sf_tasks resides). " | |
| "If not provided, the root is detected relative to the current working directory.", | |
| ) | |
| parser.add_argument( | |
| "--data-path", | |
| dest="data_path", | |
| help="Explicit path to tasks.json or to its parent directory. " | |
| "This overrides --mcdr-root and automatic detection.", | |
| ) | |
| args = parser.parse_args() | |
| # Determine data path logic | |
| cwd = os.getcwd() | |
| mcdr_root = None | |
| data_path = None | |
| # Highest priority: explicit data path | |
| if getattr(args, "data_path", None): | |
| provided = os.path.abspath(args.data_path) | |
| if os.path.isdir(provided): | |
| data_path = os.path.join(provided, "tasks.json") | |
| else: | |
| data_path = provided | |
| # Next: explicit MCDR root | |
| elif getattr(args, "mcdr_root", None): | |
| mcdr_root = os.path.abspath(args.mcdr_root) | |
| else: | |
| # Strategy: Try to find MCDR root relative to current location | |
| # Priority 1: If running inside plugin source folder (contains mcdreforged.plugin.json) | |
| if os.path.exists(os.path.join(cwd, "mcdreforged.plugin.json")): | |
| mcdr_root = os.path.abspath(os.path.join(cwd, "../..")) | |
| # Priority 2: If running in a directory that contains sf_tasks in parent (e.g. plugins dir) | |
| elif os.path.exists(os.path.join(cwd, "../sf_tasks")): | |
| mcdr_root = os.path.abspath(os.path.join(cwd, "..")) | |
| # Priority 3: If sf_tasks exists in current directory, treat cwd as root | |
| elif os.path.exists(os.path.join(cwd, "sf_tasks")): | |
| mcdr_root = cwd | |
| # Priority 4: If sf_tasks exists in ../.., use it as a convenience | |
| elif os.path.exists(os.path.join(cwd, "../../sf_tasks")): | |
| mcdr_root = os.path.abspath(os.path.join(cwd, "../..")) | |
| else: | |
| # Final fallback: assume current working directory is the root | |
| mcdr_root = cwd | |
| if data_path is None: | |
| data_path = os.path.join(mcdr_root, 'sf_tasks', 'tasks.json') |
| if not success: | ||
| source.reply(Utils.error_msg(server, 'sakuraflow.msg.remove_failed', context['value'])) | ||
| return |
There was a problem hiding this comment.
The indentation in this error branch is inconsistent with the rest of the file (extra leading space). Re-indent to the standard 4-space level for clarity and to avoid style/lint issues.
|
|
||
| # 尝试判断是否为纯数字(翻页) | ||
| is_page = False | ||
| page = 1 |
There was a problem hiding this comment.
| def release(self): | ||
| try: | ||
| os.remove(self.lock_file) | ||
| except OSError: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
This pull request introduces several major improvements to the Sakura Flow plugin, focusing on enhanced usability, better integration, and improved maintainability. The most important changes include the addition of a CLI entry point, a comprehensive overhaul of documentation, migration of language keys for consistency, improved plugin initialization and command registration, and the setup of CI/CD workflow for automated testing and releases.
New features and usability improvements:
__main__.py, enabling command-line management of tasks outside of Minecraft. This allows users to interact with their todo lists directly from the terminal, supporting improved automation and accessibility.README.md, including a quick start guide, detailed command reference, attribute cheat sheet, and a full example workflow. This makes the plugin much easier for new users to understand and adopt.Codebase and localization enhancements:
lang/zh_cn.jsonfrom thetodo.namespace tosakuraflow., and added new keys for search functionality and error handling. This improves consistency and prepares for future extensibility.sakura_flow/__init__.pyto use a controller abstraction and updated command registration to use a newregister_mcdr_commandsfunction. Data storage now defaults to the MCDR root'ssf_tasksdirectory for better separation and reliability.Infrastructure and compatibility:
.github/workflows/test.yml, automating linting, testing, building, and release publishing. This ensures code quality and streamlines the release process.mcdreforged.plugin.jsonto reflect the new repository link and lowered the minimum required MCDReforged version for broader compatibility.These changes collectively make Sakura Flow more robust, user-friendly, and maintainable, while also laying the groundwork for future enhancements and easier collaboration.