-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Add Terminus Persistence package #9234
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Add Terminus Persistence package #9234
Conversation
|
Please follow the checklist in the pull request template. There seems to be a conflict on the file you changed, please merge/rebase to resolve that. Since you're writing a file to user-space, would be good to document that so users know where that file came from and why. I wouldn't normally recommend adding menu items for commands your package doesn't own (toggle_terminus_panel). Better to leave that to the owning package and whatever preferences the user has around that. The toggle command of your package seems more like a preference, and can be put in the package settings menu instead of the view menu. |
|
Significant reformatting of json files just to add a package is probably undesirable. |
4bb60ec to
5ffb4a6
Compare
Package ReviewChannel DiffRemoved (none), changed (none), added Terminus Persistence. Review for Terminus Persistence 0.1.0 |
repository/t.json
Outdated
| "description": "Automatic state persistence for Sublime Text Terminus plugin", | ||
| "author": "gabriel-lody", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "description": "Automatic state persistence for Sublime Text Terminus plugin", | |
| "author": "gabriel-lody", |
The GitHub repo provides this information so it can be omitted here.
|
Please respond to the feedback |
This package provides automatic state persistence for the Terminus plugin, remembering panel visibility between Sublime Text sessions. Package entry placed in correct alphabetical position (after TerminalView, before termX). Changes from previous submission: - Rebased on latest upstream/master - Minimized JSON changes (12 lines only) - Fixed alphabetical ordering (Terminus Persistence before termX) - Repository updated with reviewer feedback: - Removed menu items for commands not owned by this package - Added documentation for user-space files created Addresses feedback from PR review by @braver and @deathaxe
5ffb4a6 to
1395c26
Compare
|
Hi @braver, thank you for the feedback! I've addressed all the points: Changes Made1. Checklist Compliance
2. User-Space File DocumentationAdded documentation in the README explaining:
3. Menu ItemsRemoved the
4. Alphabetical Ordering (Schema Test Fix)Fixed the package entry position - now correctly placed after "TerminalView" and before "termX" in alphabetical order. All schema tests pass locally (9876 tests OK). Let me know if there's anything else needed! |
No you didn't. Or more likely, whatever ai/agent you used didn't. Anyway, whatever, it's fine. Some final remarks: What does this checkbox property do? I don't believe that's a thing you can set in a menu (docs). I looks like your settings file still has the state data in it, but it doesn't look like you're still using that. |
Package Information
Name: Terminus Persistence
Repository: https://github.com/gabriel-lody/terminus-persistence
Description: Automatic state persistence for Sublime Text Terminus plugin
Checklist
Description
This plugin automatically remembers whether the Terminus terminal panel
was open between Sublime Text sessions and restores it on startup.
Features:
Dependencies: