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 me.jtheoof.swappy #4717

Open
wants to merge 11 commits into
base: new-pr
Choose a base branch
from
Open

Add me.jtheoof.swappy #4717

wants to merge 11 commits into from

Conversation

zefr0x
Copy link

@zefr0x zefr0x commented Nov 20, 2023

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 have built and tested the submission locally.
  • 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. (If not, explain why.)

@zefr0x
Copy link
Author

zefr0x commented Nov 20, 2023

bot, build me.jtheoof.swappy

@flathubbot
Copy link

Queued test build for me.jtheoof.swappy.

@flathubbot
Copy link

Started test build 82734

@flathubbot
Copy link

Build 82734 failed

@zefr0x
Copy link
Author

zefr0x commented Nov 20, 2023

bot, build me.jtheoof.swappy

@flathubbot
Copy link

Queued test build for me.jtheoof.swappy.

@flathubbot
Copy link

Started test build 82735

@flathubbot
Copy link

Build 82735 failed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file is never installed.

also it should be upstreamed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a PR: jtheoof/swappy#158

I hope the developer will response soon.

me.jtheoof.swappy.yaml Outdated Show resolved Hide resolved
@@ -0,0 +1,29 @@
id: me.jtheoof.swappy
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jtheoof.me doesn't seem to have a website. it's hard to verify it's actually valid.

Copy link
Author

@zefr0x zefr0x Nov 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used in upstream: https://github.com/jtheoof/swappy/blob/596b9a8b17234303a9d4d2ea1d3107cb5442253e/src/application.c#L948

Should I use io.github.jtheoof.swappy instead?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then you should ask them at the same time as you contact them.

@zefr0x
Copy link
Author

zefr0x commented Nov 20, 2023

bot, build me.jtheoof.swappy

@flathubbot
Copy link

Queued test build for me.jtheoof.swappy.

@flathubbot
Copy link

Started test build 82754

@flathubbot
Copy link

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

flatpak install --user https://dl.flathub.org/build-repo/65388/me.jtheoof.swappy.flatpakref

me.jtheoof.swappy.metainfo.xml Show resolved Hide resolved
me.jtheoof.swappy.yaml Outdated Show resolved Hide resolved
me.jtheoof.swappy.yaml Outdated Show resolved Hide resolved
@bbhtt
Copy link
Contributor

bbhtt commented Nov 21, 2023

bot, build me.jtheoof.swappy

@flathubbot
Copy link

Queued test build for me.jtheoof.swappy.

@flathubbot
Copy link

Started test build 82797

@flathubbot
Copy link

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

flatpak install --user https://dl.flathub.org/build-repo/65431/me.jtheoof.swappy.flatpakref

@bbhtt
Copy link
Contributor

bbhtt commented Nov 21, 2023

Please test the above build

@bbhtt
Copy link
Contributor

bbhtt commented Nov 21, 2023

Looks like copying the image does not work. It depends on wl-clipboard https://github.com/jtheoof/swappy/blob/596b9a8b17234303a9d4d2ea1d3107cb5442253e/src/clipboard.c#L35 can you add these in the manifest and check if it works?

@zefr0x
Copy link
Author

zefr0x commented Nov 21, 2023

It works without it actually, but when you close the app it will be lost from the clipboard. (The normal behavior for any application, without a clipboard manager in your desktop)

With wl-clipboard it will be available even after closing the application.

This should only be possible in wlroots based Wayland compositors and KDE:
https://wayland.app/protocols/wlr-data-control-unstable-v1
Basically it's a hole in Wayland's security.

I think with a clipboard manager it will not make any difference but I need to test it in a real desktop environment since I don't use any clipboard manager.

@hfiguiere
Copy link
Contributor

https://wayland.app/protocols/wlr-data-control-unstable-v1

This is not availble with gnome-shell.

@bbhtt
Copy link
Contributor

bbhtt commented Nov 21, 2023

If it expects to use wl-copy/paste to persistently copy-paste, I think it'd be better to give it that.

It works without it actually, but when you close the app

Yes.

Another thing, the desktop file has NoDisplay=true, but this is a type=desktop-application in metainfo, not a console one. This would be confusing to people installing the application for the first time.

@zefr0x
Copy link
Author

zefr0x commented Nov 21, 2023

Another thing, the desktop file has NoDisplay=true, but this is a type=desktop-application in metainfo, not a console one. This would be confusing to people installing the application for the first time.

Should I change it to console-application in metainfo file?

It's a weird application, we can't open it without passing a file to it (so we should not show it in applications menu), but it has a GUI. It should be visible from context menu in file managers when opening an image.

@zefr0x
Copy link
Author

zefr0x commented Nov 21, 2023

It seems it relay on FontAwesome v5 in buttons to display UTF-8 icons, so I did add it.

https://github.com/jtheoof/swappy/blob/596b9a8b17234303a9d4d2ea1d3107cb5442253e/res/style/swappy.css#L2

@barthalion
Copy link
Member

bot, build me.jtheoof.swappy

@flathubbot
Copy link

Queued test build for me.jtheoof.swappy.

@flathubbot
Copy link

Started test build 83049

@flathubbot
Copy link

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

flatpak install --user https://dl.flathub.org/build-repo/65685/me.jtheoof.swappy.flatpakref

@bbhtt
Copy link
Contributor

bbhtt commented Nov 22, 2023

It's a weird application, we can't open it without passing a file to it (so we should not show it in applications menu), but it has a GUI. It should be visible from context menu in file managers when opening an image.

I was hoping there would be some way to spawn an empty GUI..

Also, the program doesn't have a close button.

@barthalion
Copy link
Member

Yeah, it doesn't look like a good fit for Flathub right now.

@bbhtt bbhtt added the blocked Pull requests that are blocked on something or won't be accepted in the currrent state label Jan 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Pull requests that are blocked on something or won't be accepted in the currrent state
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants