-
Notifications
You must be signed in to change notification settings - Fork 56
Added Launch Application plugin #607
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: develop
Are you sure you want to change the base?
Added Launch Application plugin #607
Conversation
leon-gh
commented
Apr 19, 2025
- This plugin can launch any application as an action
* This plugin can launch any application as an action
| ) | ||
|
|
||
| if not self.is_valid(): | ||
| raise GremlinError(f"{self.application_to_launch} does not exists or is not accessible.") |
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.
If we throw an error for this here it should be a ProfileException. It's one of the very few specific ones that exists. Not sure we want to crash Gremlin for this problem though as the is_valid should result in it not being run and the functor will also bail on running it.
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.
n/p, changed to ProfileError. I think loading a bad profile could lead to unexpected behavior during use, so it's better to warn a user to fix their profile before activating it. Let me know how you want it done.
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.
Yeah that can work though may also lead to "broken" profiles that trip people in how to fix this, though that's something in general that needs to be looked at at some point, i.e. how to handle "bad" profiles.
| text: "Application to launch" | ||
| } | ||
|
|
||
| TextField { |
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.
Question here is maybe if you want to allow the user to manually modify the executable or not. If not the text field can be made non-editable. If on the other hand they should edit it, maybe we can use a verification callback slot in the model with file_exists_and_is_accessible to ensure the final file is valid.
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.
I'd like to give the user ability to edit - makes it easier to copy-paste stuff, so you don't have to edit xml every time.
Let me know how you want it done and I'll modify it, if needed.
Updated TextField to trigger action updates only onEditingFinished and fixed FileDialog.onAccepted event to set action.application_path.
Also added throwing GremlinError in _set_application_path if file does not exist, but it shows in console only, not a pop-up as it would be nice.
Any suggestions?
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.
That makes sense and I agree that it makes sense to let users modify by hand the content. I had hoped that TextLField had a good validation system, but the one it has https://doc.qt.io/qt-6/qml-qtquick-textinput.html#validator-prop is limited to a few presets that are not useful to us. So the way I'd do this is:
- define a property like
isInputValidin the model which checks if the file exists - change the outline based on that boolean, e.g. red if not valid and blue or nothing when it is valid
- whenever the content changes the connections via signals should let QML know the value changed and thus update the UI
That way you don't have to throw an exception, which would be awkward while editing, but the user would still get a visual indication that something is wrong, since we're all trained to spot "red fields"
The whole exception and popup stuff used to work but it's misbehaving but I haven't looked into it and it may also only misbehave when run via the IDE but be ok when run as an application. Though I wouldn't inform the user about this issue with an exception as that'd get really annoying quickly. To me exceptions are really "something is brokenly wrong", like vJoy device missing etc.
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.
I would not want to throw the exception either, but just some kind of pop-up or something to alert the user that something is wrong. I like red/blue outline, but I'm afraid users might not see it unless it's blinking or so, and it's so easy to "go to the next" button then realizing the old one didn't save your setting.
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.
Imo, there is only so much helping one can do. If someone can't see a clearly marked error area, that's on them; I wouldn't want to subject competent users to annoying modal windows because of that. Exceptions in Gremlin are really for things that can't possibly work without being addressed, whereas here, it just won't execute a program.
Something down the road to have is a display/report thing that much more clearly shows/highlights issues with a configuration so that people have a way to see issues. Easiest instance of that is a warning sign on the inputs (again not all are visible all the time) and better is some side panel that shows error things like most IDEs do these days.