Skip to content
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

XdgDesktopFile: Adds Exec fallback when DBusActivatable fails #80

Merged
merged 2 commits into from
Mar 27, 2016

Conversation

luis-pereira
Copy link
Member

We fallback to use the Exec key when the DBusActivatable fails.
Closes lxqt/lxqt#967.

We fallback to use the Exec key when the DBusActivatable fails.
Closes lxqt/lxqt#967.
@jleclanche
Copy link
Member

gtm

@palinek
Copy link
Contributor

palinek commented Mar 19, 2016

I was also considering this, but it violates the specification "If the value is true then implementations should ignore the Exec key and send a D-Bus message to launch the application."

@luis-pereira
Copy link
Member Author

@palinek But the specification also says:

Even if DBusActivatable is true, Exec should be specified for compatibility with implementations that do not understand DBusActivatable.

@palinek
Copy link
Contributor

palinek commented Mar 19, 2016

that do not understand DBusActivatable

But we do.

@luis-pereira
Copy link
Member Author

Yes. But how do libqtxdg determine if a given environment understands or not DBusActivatable ?

@palinek
Copy link
Contributor

palinek commented Mar 21, 2016

Yes. But how do libqtxdg determine if a given environment understands or not DBusActivatable ?

Libqtxdg is the environment/implementation here, not?

@luis-pereira
Copy link
Member Author

Sure. Not exactly what I meant.
What I meant is that even the standard guys do not entirely follow the standard.
When DBusActivatable was introduced at least desktop-file-validate should got update to warn about the presence of - in the filename.

We strictly follow the standard and get a fail (user viewpoint).

@palinek
Copy link
Contributor

palinek commented Mar 22, 2016

We strictly follow the standard and get a fail (user viewpoint).

From user point of view it is definitely better to try the Exec as a fallback if DBus fails... there is no doubt.

Let's do it. But IMO it has to be documented somehow (comment in code, readme?), that we are violating the standard for a good reason.

@luis-pereira
Copy link
Member Author

@palinek I do agree with the documentation). Will write it.

@luis-pereira
Copy link
Member Author

@palinek Documented at d019654. Take a look.

@palinek
Copy link
Contributor

palinek commented Mar 22, 2016

It's a bit too verbose... But the fallback is executed for whatever "fail" reason (no dbus at all, not properly configured the dbus service for the applicaion...), not only the invalid name.

@luis-pereira
Copy link
Member Author

@palinek It's verbose, but IMO it deserves a very good explanation.
The fallback is now only executed when there is an invalid dbus object path.

@palinek
Copy link
Contributor

palinek commented Mar 23, 2016

The fallback is now only executed when there is an invalid dbus object path.

Hm... why? From end user POI it doesn't matter what triggered the error. With my previous note I proposed a change in comment/doc, not the change of the previous logic.

@luis-pereira
Copy link
Member Author

@palinek I inferred from your word that you only agreed in doing it in that special case

@palinek
Copy link
Contributor

palinek commented Mar 23, 2016

that you only agreed in doing it in that special case

Sorry for confusing you 😄

We are deliberately violating the standard. The minimum we can do is to
document it properly.
@luis-pereira
Copy link
Member Author

@palinek No problem. Updated.

@paulolieuthier
Copy link
Contributor

GTM.

@palinek
Copy link
Contributor

palinek commented Mar 27, 2016

GTM

@luis-pereira luis-pereira merged commit 4fb6316 into master Mar 27, 2016
@luis-pereira luis-pereira deleted the dbusactivatable-add-fallback branch March 27, 2016 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants