Add open in external editor QMenu item#118
Conversation
Add a QMenu Item, that will open the current repo inside the configured external edito. Add a new icon, for the QMenu Item.
jorio
left a comment
There was a problem hiding this comment.
Good idea! A few notes:
-
Since the menu item contains the name of the editor, it would be great to sync the label when the user changes their editor setting. See
PrefEffects.RebuildMenu. -
If the user hasn't configured an editor (
settings.prefs.externalEditoris empty), GitFourchette starts the system's default application associated with the path that's being opened. When opening a directory, the system default app is a file manager, not an editor! This is confusing, so I suggest stopping with an error in this case. You can usesetUpToolCommand(..., "externalEditor")for a ready-made dialog that prompts the user to set up their editor. -
Please add a test so we don't break this feature in the future. You can flesh out
testTabBarActions, where similar actions are already being tested. You can set the editor toeditor-shim.pywhich spits the arguments back to a scratch file. (Open Terminal is already tested like this.) -
I make all icons by hand for consistency, and to skirt licensing issues. I'll push one when I have time. Maybe
prefs-diffis a good enough placeholder in the meantime. -
Editor-specific icons would be nice, but yes, let's think about those later. Maybe we can use something like
QIcon.fromTheme("kate"), which also works with Flatpak IDs. We could factor the "interesting token extraction" out of ToolPresets.getCommandName to get the correct token to pass to QIcon.fromTheme.
|
I guess I changed all of your remarks. |
|
Thanks! Can we also ensure that the workdir was actually passed to the editor? Something like I'd also like to cover the code path when the user has no editor set. You can clear the editor pref and do
Unrelated, don't worry! I guess the timeout values aren't generous enough for when the CI runner happens to be under heavier load than normal. |
|
I noticed two things while working on the tests: 1. VS Code Formatting and RuffRuff seems to be configured primarily for linting. But because I have format-on-save enabled globally, it caused some formatting conflicts on my end. To prevent this, we could add the following to "[python]": {
"editor.formatOnSave": false
}Alternatively, we could enforce a specific formatting style so that running Ruff does not conflict with existing code? 2. Offscreen Test Execution on WaylandWhen running tests inside VS Code on Wayland, the Is it intentional that tests do not run offscreen when executed without debugging? I noticed To resolve this, I propose unifying this configuration at the top of # --- Force Qt to run offscreen for all automated tests ---
os.environ["QT_QPA_PLATFORM"] = "offscreen"
# ---------------------------------------------------------- |
|
Thank you for the PR and your notes!
Sure! Feel free to submit a PR to improve the VS Code config. I supplied a basic set of config files for VS Code, but it's not my primary editor so there's probably room for improvement.
Yes, ruff is only used for linting for now. Long term, enforcing a formatting style might be good to do. The current style isn't 100% consistent so I'll probably still have to format the entire codebase at some point. I'll think about it.
Yeah, Wayland introduces a few pitfalls when tests are run visually, due to their restrictions on mouse cursor control (compared to X11 or macOS). That's why the offscreen mode is the only "source of truth", and visual tests are not reliable. (For what it's worth, the workaround on Wayland is to position the mouse cursor on top of the test window, not over another application.)
When I flesh out a new test, I like to run it visually so I can see the state of the UI. In doing so, I sometimes use pauseDialog() to explore the UI without locking up the application. When it's time to check that everything actually works, I use
I don't think we should override the developer's env variables. This would break the workflow I described above ;) |
Description
This PR adds a new "Open in external editor" option to the
Repomenu (QMenu).🛠️ Changes Made
Repomenu.❓ Questions & Considerations
README.md(or aCONTRIBUTING.md) outlining which icon libraries contributors should stick to? This would help keep the UI consistent and clarify licensing for future changes.💡 Future Idea / RFC (Request for Comments)
I originally considered making the menu icon dynamic based on the user's selected editor (e.g., displaying a VS Code icon for VS Code, a Neovim icon for NVIM, etc.).
However, the current
ActionDefclass doesn't support this dynamic behavior out of the box. I opted not to alter the architecture ofActionDefwithout your guidance. Let me know if you would like to see this implemented, and I can open a separate follow-up PR for it!📷 Visuals
The screenshot below demonstrates the new
QMenuitem with VS Code configured as the selected external editor: