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

Yelp help not launching from help menu in EasyEffects on Manjaro #2046

Open
b14ckw1d0w opened this issue Dec 24, 2022 · 27 comments
Open

Yelp help not launching from help menu in EasyEffects on Manjaro #2046

b14ckw1d0w opened this issue Dec 24, 2022 · 27 comments

Comments

@b14ckw1d0w
Copy link

I cannot get the documentation to load via the in-app menu. I am running the latest Manjaro with KDE using the default repo version of EasyEffects and have Yelp installed.

K Help Centre is instead loaded via the in-app menu or by pressing F1 instead of Yelp with a page saying the documentation is missing.

It has taken me a few days to figure out that if I just load Yelp on its own then I can access the documentation.

I am unsure if this is a bug with the software or the fact that I am running Manjaro KDE.

@wwmm
Copy link
Owner

wwmm commented Dec 24, 2022

I am unsure if this is a bug with the software or the fact that I am running Manjaro KDE.

Probably some unexpected interaction when using KDE considering yelp is being used on GNOME. I was not aware yelp was not being used on KDE. That is unfortunate.

I am not sure if there is a way to force GTK to call yelp. At this moment we just ask gtk to open the help

gtk_show_uri(gtk_application_get_active_window(GTK_APPLICATION(gapp)), "help:easyeffects",

Maybe there is a way to tell gtk_show_uri that yelp has to be used.

@wwmm
Copy link
Owner

wwmm commented Dec 24, 2022

Maybe there is a way to tell gtk_show_uri that yelp has to be used.

At least in its docs I could not see a way. All it says is

This function launches the default application for showing a given uri, or shows an error dialog if that fails.

. The problem is that the default app for this in KDE will never be yelp =/.

@medmedin2014
Copy link

@wwmm Why not convert EasyEffects docs to a standard format like HTML instead of those ambiguous .page files, so Khelpcenter can view it ?

@wwmm
Copy link
Owner

wwmm commented Jan 19, 2024

Why not convert EasyEffects docs to a standard format like HTML instead of those ambiguous .page files, so Khelpcenter can view it ?

No special reason other than the fact there were straightforward examples showing how to handle translations in Meson with the Mallard xml format. If there is a simple way to handle translations with plain html and someone is willing to rewrite everything I do not see any problem.

@medmedin2014
Copy link

@wwmm It seems you can use yelp-tools to convert whole .page files to .html

yelp-build html -o out-dir in-dir

I tried to create directory in /usr/share/doc/easyeffects with those generated HTML files, but sadly they are still not found when khelpcenter launch.

@wwmm
Copy link
Owner

wwmm commented Jan 19, 2024

I tried to create directory in /usr/share/doc/easyeffects with those generated HTML files, but sadly they are still not found when khelpcenter launch.

Probably because it needs some kind of instruction to know it has to load html from that path. Or it is just a matter of figuring out where it searches for docs. For yelp the page files are installed inside /usr/share/help/C/easyeffects/. What if you put the html there?

As far as our Meson scripts go we do not do anything special for yelp https://github.com/wwmm/easyeffects/blob/master/help/meson.build. We just tell Meson to do its thing. I wonder if under the hoods it is doing more than just copying files to /usr/share/help/C/easyeffects/.

It seems you can use yelp-tools to convert whole .page files to .html

But what happens to the translations?

@medmedin2014
Copy link

Probably because it needs some kind of instruction to know it has to load html from that path. Or it is just a matter of figuring out where it searches for docs. For yelp the page files are installed inside /usr/share/help/C/easyeffects/. What if you put the html there?

I got the same error
Screenshot_20240119_153725
But if I run khelpcenter /usr/share/help/C/easyeffects/index.html it finds the docs
Screenshot_20240119_153914

But what happens to the translations?

I'm not sure, I just took the english content of /usr/share/help/C/easyeffects and convert it to HTML.

@wwmm
Copy link
Owner

wwmm commented Jan 19, 2024

I'm not sure, I just took the english content of /usr/share/help/C/easyeffects and convert it to HTML.

I think translations are probably not going to work this way. Under the hoods Meson's module for gnome is doing the necessary calls to create the translation files for the yelp pages. We will have to figure out how to make Meson handle docs and translation in a way khelpcenter finds them.

@vchernin
Copy link
Contributor

I was under the impression that we are no longer translating the docs due to #1108?

Instead of trying to make the docs work in yelp/khelpcenter (I still have no idea why the help pages don’t work in flatpak), we could simply point everyone to the website https://wwmm.github.io/easyeffects/. It should be possible to change the current docs ci job to upload the docs from multiple versions.

e.g.
https://wwmm.github.io/easyeffects/7.1.3/page1.html
and
https://wwmm.github.io/easyeffects/7.1.2/page2.html

@wwmm
Copy link
Owner

wwmm commented Jan 19, 2024

I was under the impression that we are no longer translating the docs due to #1108?

Oh... I forgot about it. So translations can be forgotten.

Instead of trying to make the docs work in yelp/khelpcenter (I still have no idea why the help pages don’t work in flatpak), we could simply point everyone to the website https://wwmm.github.io/easyeffects/.

It is something to consider. It probably can be done just changing the url we already give to gtk.

@medmedin2014
Copy link

medmedin2014 commented Jan 19, 2024

@vchernin English documentation is quite sufficient, and it's better to focus the effort on a high quality version of doc instead of multiple versions.

The offline doc is necessary for any app that run on any desktop.

@wwmm As an alternative, why instead of calling
gtk_show_uri(gtk_application_get_active_window(GTK_APPLICATION(gapp)), "help:easyeffects",

We execute directly the command yelp help:easyeffects

The command will never fail since yelp is marked as a required dependency for easyeffects package on many distributions.

Edit: Sorry, it seems on Arch based distros it's marked as optional. So for the command to work, yelp should be changed to be a required dependency.

@medmedin2014
Copy link

I created a bug on KDE bug tracker, may be KDE team will come with a solid solution like how they always do.

@vchernin
Copy link
Contributor

@wwmm As an alternative, why instead of calling gtk_show_uri(gtk_application_get_active_window(GTK_APPLICATION(gapp)), "help:easyeffects",

We execute directly the command yelp help:easyeffects

This also works properly in flatpak builds, since it doesn't try to open easyeffects' help pages from the host yelp.

@wwmm
Copy link
Owner

wwmm commented Jan 19, 2024

We execute directly the command yelp help:easyeffects

The problem is security. Direct calls to executables can open the window for malicious software if not properly done. I prefer to not have this kind of headache... And this keeps us in the same page as other apps. I do not remember a gtk app that directly calls yelp. All of them delegate this task to the toolkit.

@wwmm
Copy link
Owner

wwmm commented Jan 20, 2024

I wonder if there is an easy way to check if the current session is a KDE session. This way we could do the system call only when running inside KDE and keeping doing what we already do for the others. It still is horrible from a security point of view but this way only one desktop environment would be vulnerable until a better solution is available.

@wwmm
Copy link
Owner

wwmm commented Feb 6, 2024

It seems that the environment variable DESKTOP_SESSION can be used to get the desktop being used by the user. @medmedin2014 can you confirm that it is set to kde when using kde? If yes we can change the invoked command based on whether we are or not in a kde session.

@wwmm
Copy link
Owner

wwmm commented Feb 6, 2024

The variable XDG_CURRENT_DESKTOP seems to also have this information. But in its case it is in upper case GNOME instead of gnome.

@wwmm
Copy link
Owner

wwmm commented Feb 6, 2024

It won't be a problem but I've noticed now that calling std::system("yelp help:easyeffects"); freezes EasyEffects until yelp is closed. What is not cool. This command will have to be called in its own thread.

@wwmm
Copy link
Owner

wwmm commented Feb 6, 2024

I have updated the master branch with the workaround. As expected clang-tidy complained about cert-env33-c but there is nothing better to do. Bringing back a huge dependency like the Boost library just to call this in a more secure way is overkill.

@medmedin2014
Copy link

@wwmm

On Wayland:

echo $DESKTOP_SESSION
plasmawayland

On X11:

echo $DESKTOP_SESSION
plasma

Both on Wayland and X11:

echo $XDG_CURRENT_DESKTOP 
KDE

@wwmm
Copy link
Owner

wwmm commented Feb 6, 2024

plasmawayland
plasma

Thanks @medmedin2014 ! I've updated our master branch with the correct values for DESKTOP_SESSION. I guess now it is just a matter of someone that uses KDE to do some tests.

@medmedin2014
Copy link

medmedin2014 commented Feb 6, 2024

@wwmm
Nate Graham from KDE team responded to my question which reliable environment variable to use to determine if the current running desktop is KDE Plasma, his answer was to use XDG_SESSION_DESKTOP with its return value KDE.

@wwmm
Copy link
Owner

wwmm commented Feb 6, 2024

Nate Graham from KDE team responded to my question which reliable environment to use to determine if the current running desktop is KDE Plasma, his answer was to use XDG_SESSION_DESKTOP with its return value KDE.

Ok. Good to know. At this moment the workaround is checking for both XDG_SESSION_DESKTOP and DESKTOP_SESSION. If one of them is set to a value related to kde the workaround is applied.

@medmedin2014
Copy link

@wwmm
Screenshot_20240206_235250
xdg_desktop_session should be XDG_SESSION_DESKTOP which returns a single value KDE, instead of XDG_CURRENT_DESKTOP which may return a colon-separated list.

@alexbramford
Copy link

@wwmm

After pulling latest changes from master, I can confirm easyeffects help is working, without freezing or interrupting Rhythmbox / pipewire.

Also, on Fedora 39 KDE I get:

05:11:39 tma7-alexb ~/temp> echo $DESKTOP_SESSION

plasmax11

05:11:46 tma7-alexb ~/temp> echo $XDG_CURRENT_DESKTOP

KDE

easy effects with yelp

esay effects help is working

My system:

05:01:41 tma7-alexb ~/temp> screenfetch -n
 alexb@tma7.spaceodyssey.2001.com
 OS: Fedora 39 ThirtyNine
 Kernel: x86_64 Linux 6.6.14-200.fc39.x86_64
 Uptime: 2d 8h 46m
 Packages: 3739
 Shell: bash 5.2.26
 Resolution: 2560x1440
 DE: KDE 5.113.0 / Plasma 5.27.10
 WM: KWin
 WM Theme: Breeze Dark
 GTK Theme: Breeze [GTK3]
 Disk: 2.3T / 2.7T (87%)
 CPU: Intel Core i7-7500U @ 4x 3.5GHz [62.0°C]
 GPU: Mesa Intel(R) HD Graphics 620 (KBL GT2)
 RAM: 11741MiB / 31842MiB

@wwmm
Copy link
Owner

wwmm commented Feb 6, 2024

xdg_desktop_session should be XDG_SESSION_DESKTOP which returns a single value KDE, instead of XDG_CURRENT_DESKTOP which may return a colon-separated list.

Is the value in upper case or lower case? In my gnome session I have XDG_SESSION_DESKTOP=gnome

@medmedin2014
Copy link

medmedin2014 commented Feb 7, 2024

@wwmm

Is the value in upper case or lower case? In my gnome session I have XDG_SESSION_DESKTOP=gnome

According to systemd's PAM and sd_session_get_desktop(3)

This field can be set freely by desktop environments and does not follow any special formatting. However, desktops are strongly recommended to use the same identifiers and capitalization as for $XDG_CURRENT_DESKTOP, as defined by the Desktop Entry Specification.

I tested the output of echo $XDG_SESSION_DESKTOP on Arch Linux/Manjaro KDE, Fedora KDE 39, Kubuntu 23.10 and openSUSE KDE Tumbleweed, and they all returned the upper case KDE.

To avoid the problem of upper or lower case, it's best to get the value then apply upper case function on it then compare it to "KDE", just to be on the safe side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants