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

Add manifest for org.kde.kdf #2780

Closed
wants to merge 4 commits into from

Conversation

flyingcakes85
Copy link

@flyingcakes85 flyingcakes85 commented Jan 18, 2022

Please confirm your submission meets all the criteria

  • I have read the App Requirements and App Maintenance pages.
  • My pull request follows the instructions at App Submission.
  • I am using only the minimal set of permissions. (If not, please explain each non-standard permission.)
  • All assets referenced in the manifest are redistributable by any party. If not, the unredistributable parts are using an extra-data source type.
  • I am an upstream contributor to the project. If not, I contacted upstream developers about submitting their software to Flathub. Link:
  • I own the domain used in the application ID or the domain has a policy for delegating subdomains (e.g. GitHub, SourceForge).
  • Any additional patches or files have been submitted to the upstream projects concerned.

Notes Dolphin manifest borrowed from https://github.com/flathub/org.kde.dolphin

Not relevant now.

Signed-off-by: Snehit Sah <snehitsah@protonmail.com>
@flyingcakes85
Copy link
Author

bot, build org.kde.kdf

@flathubbot
Copy link

Queued test build for org.kde.kdf.

@flathubbot
Copy link

Started test build 75179

Signed-off-by: Snehit Sah <snehitsah@protonmail.com>
@flyingcakes85
Copy link
Author

Dolphin (and its dependencies) removed from manifest.

@flyingcakes85
Copy link
Author

bot, build org.kde.kdf

@flathubbot
Copy link

Queued test build for org.kde.kdf.

@flathubbot
Copy link

Build 75179 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/73031/org.kde.kdf.flatpakref

@flathubbot
Copy link

Started test build 75182

@flathubbot
Copy link

Build 75182 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/73034/org.kde.kdf.flatpakref

@nedrichards
Copy link
Member

@flathub/kde FYI

I wonder about the open in file manager stuff. Without a call to flatpak-spawn (and the necessary permission) it's not going to work that well, but also doing it seamlessly would ideally require some changes in the main codebase. What does the rest of the KDE team think?

@tsdgeos
Copy link

tsdgeos commented Jan 19, 2022

My opinion is that we need to patch kdf to disable the feature of letting you chose a random file manager when running in flatpak and instead just a QDesktopServices::openUrl call

@flyingcakes85
Copy link
Author

My opinion is that we need to patch kdf to disable the feature of letting you chose a random file manager when running in flatpak and instead just a QDesktopServices::openUrl call

I'll take care of KDF source code, and update this pull request with a patch. Till then this PR is on hold.

@flyingcakes85 flyingcakes85 marked this pull request as draft January 20, 2022 04:51
@travier
Copy link
Member

travier commented Jan 20, 2022

CC @flathub/kde

Signed-off-by: Snehit Sah <snehitsah@protonmail.com>
@flyingcakes85
Copy link
Author

bot, build org.kde.kdf

@flathubbot
Copy link

Queued test build for org.kde.kdf.

@flyingcakes85 flyingcakes85 marked this pull request as ready for review January 29, 2022 05:25
@flathubbot
Copy link

Started test build 76314

@flathubbot
Copy link

Build 76314 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/74185/org.kde.kdf.flatpakref

@flyingcakes85
Copy link
Author

flyingcakes85 commented Jan 29, 2022

@travier

Opening in file manager still doesn't work on my machines. The only scenario where I can get it to work is when having Thunar as default file manager and opening a thumb drive. Opening internal drives don't work. And with Dolhpin, nothing opens.

Could someone please test the build from flathubbot in last comment? Install it, and from KDF, right click -> open in file manager; and check if it opens.

@nedrichards
Copy link
Member

Absolutely does not work on Fedora Silverblue 35 under GNOME.

@tsdgeos
Copy link

tsdgeos commented Jan 30, 2022

Seems you have found a bug in Qt's portal integration, looping @grulja in

We're doing QDesktopServices::openUrl("file:///"), i.e. open the root folder, but Qt is xdgDesktopPortalOpenFile, trying to open a fd out of / and passing it to the portal OpenFile method

It is my understanding the portal OpenDirectory method would make more sense in this case

@travier
Copy link
Member

travier commented Feb 2, 2022

Tested this one and it looks like it needs --device=all to figure out the names and mount points, etc.
Apart from that, it worked for me so LGTM. If it does not work well for others then let's merge it as Beta and figure things out progressively?

@tsdgeos
Copy link

tsdgeos commented Feb 2, 2022

going to beta seems reasonable to me

Signed-off-by: Snehit Sah <snehitsah@protonmail.com>
@flyingcakes85
Copy link
Author

bot, build org.kde.kdf

@flathubbot
Copy link

Queued test build for org.kde.kdf.

@flathubbot
Copy link

Started test build 76861

@flathubbot
Copy link

Build 76861 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/74733/org.kde.kdf.flatpakref

@grulja
Copy link

grulja commented Feb 3, 2022

Seems you have found a bug in Qt's portal integration, looping @grulja in

We're doing QDesktopServices::openUrl("file:///"), i.e. open the root folder, but Qt is xdgDesktopPortalOpenFile, trying to open a fd out of / and passing it to the portal OpenFile method

It is my understanding the portal OpenDirectory method would make more sense in this case

Yes and I think back then when I implemented this method didn't exist. I can try to look into this once I find some time or if anyone wants to do it, I will happily help.

It needs to just check here [1] that the URL is a directory and make a different call.

[1] - https://invent.kde.org/qt/qt/qtbase/-/blob/dev/src/gui/platform/unix/qgenericunixservices.cpp#L301

@travier
Copy link
Member

travier commented Feb 8, 2022

I think we should merge this one as Beta as it works. We will fix the remaining issues as we go.

@travier
Copy link
Member

travier commented Feb 8, 2022

CC @flathub/kde

@tsdgeos
Copy link

tsdgeos commented Feb 8, 2022

makes sense

@barthalion
Copy link
Member

/merge:beta

@flathubbot
Copy link

A repository for this submission has been created: https://github.com/flathub/org.kde.kdf

You will receive an invitation to be a collaborator which will grant you write access to the repository above. The invite can be also viewed here.

If you have never maintained an application before, common questions are answered in the app maintenance guide.

Thanks!

@flathubbot flathubbot closed this Feb 9, 2022
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.

None yet

7 participants