-
-
Notifications
You must be signed in to change notification settings - Fork 116
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
[GTK4 Prep] Move External App Actions Logic To Utils #1455
[GTK4 Prep] Move External App Actions Logic To Utils #1455
Conversation
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.
Seems sensible. Just one question and a couple comments about extra whitespace
b57bf1a
to
f1d903e
Compare
I've made new changes that should resolve all the issues now. |
src/Utils.vala
Outdated
var menu = new Gtk.Menu (); | ||
|
||
try { | ||
var contracts = Granite.Services.ContractorProxy.get_contracts_by_mime (file_type); |
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.
Should this be https://valadoc.org/granite-7/Granite.Services.ContractorProxy.get_contracts_for_file.html so we don't have to manually get file type?
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 makes the code more concise. Thanks!
src/Utils.vala
Outdated
var file = GLib.File.new_for_path (path); | ||
|
||
try { | ||
var contracts = Granite.Services.ContractorProxy.get_contracts_by_mime (file_type); |
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.
Same here
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.
This has been updated now too.
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.
Nice work!
Part of of "Replace Gtk.MenuItem with GLib.MenuModel" task in #1157