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

Upstream repository #8

Open
gfgit opened this issue Feb 20, 2024 · 26 comments
Open

Upstream repository #8

gfgit opened this issue Feb 20, 2024 · 26 comments
Labels
help wanted Extra attention is needed

Comments

@gfgit
Copy link
Member

gfgit commented Feb 20, 2024

Hello,
I've looked around but could not find an upstream repository for this fork.
However there are many similar repositories:

  1. https://invent.kde.org/plasma/plasma-workspace/-/tree/master/libdbusmenuqt
  2. https://launchpad.net/libdbusmenu-qt
  3. https://github.com/deepin-community/libdbusmenu-qt
  4. https://github.com/qtilities/libdbusmenu-qtilities
  5. https://github.com/OpenMandrivaAssociation/libdbusmenu-qt/tree/master (contains patches)
  6. https://github.com/desktop-app/libdbusmenu-qt (archived)
  7. https://github.com/a17r/libdbusmenuqt
  8. archlinux/svntogit-packages@ee7106f

Would it be worth to make a common repository which accommodates all use cases?
I bet KDE fixes could be used on LXQt too.
We could ask KDE to extract this library from inside Plasma Workspace repository and keep it as standalone repository to which we could collaborate without needing to keep a fork.
And then maybe notify other forks about this new opportunity.

What do you think?

@gfgit gfgit added the help wanted Extra attention is needed label Feb 20, 2024
@gfgit
Copy link
Member Author

gfgit commented Feb 20, 2024

Relevant KDE discussion: https://phabricator.kde.org/T13138

Last post is from @yan12125 but it was left unanswered. Maybe we could open a Gitlab issue on Plasma Workspace repository and restart the conversation

@stefonarch
Copy link
Member

stefonarch commented Feb 20, 2024

It's in the Readme, it's a fork from 6) which is archived.
4) is a fork from this.

@gfgit
Copy link
Member Author

gfgit commented Feb 20, 2024

Ok but I think it's still better to share effort with KDE devs than having another fork which possibly tries to fix same issues

@stefonarch
Copy link
Member

I've no strong opinion about that, but we already depend on KF6. The idea afaik was to have full control over it.

@tsujan
Copy link
Member

tsujan commented Feb 20, 2024

We could ask KDE to extract this library...

IMHO, being too dependent on KDE codes isn't a good approach. KDE devs have done and are doing a great job, but since their focus is on Plasma and their manpower doesn't seem proportional to the amount of works they do, sometimes they may forget that KF is also used outside Plasma (that has happened).

Here, we already have what we need. If KDE makes fixes that are usable to us, we could adapt them.

@redtide
Copy link

redtide commented Feb 26, 2024

Would it be worth to make a common repository which accommodates all use cases?

You looks like me some months ago, same idea :D
though I don't think KDE people care about it.

I made the Qtilities version for that reason (though it seems the usual https://xkcd.com/927/ case..), other than needed because my tray volume app (a stand alone version of lxqt-panel volume plugin), but found not much interest, except by QASTools developer; we would like to have wheel support over tray icon and the only solution is using StatusNotifierItem together with that library for the tray context menu. Note that KStatusNotifierItem has it incorporated in the same code, so that makes me think they are not going to need it as separated stand alone library.

libdbusmenu-qt library is very old, AFAIK was developed more than 10 years ago by Aurélien Gâteau on a bzr repository and changed repositories and maintainers over time.

@gfgit
Copy link
Member Author

gfgit commented Feb 26, 2024

Sad story. I know this was optimistic and not real world scenario.
Still I don't think KDE devs have so narrow vision, caring only about KDE.
Anyway it's just a cool idea in comunity spirit.
@yan12125 what's you opinion on this?

@redtide
Copy link

redtide commented Feb 26, 2024

Sad story. I know this was optimistic and not real world scenario.
Still I don't think KDE devs have so narrow vision, caring only about KDE.
Anyway it's just a cool idea in comunity spirit.

What I said it's just my own opinion, I might be wrong, you can just ask them as you said. On the other hand it was also my idea initially, I thought to ask by opening an issue here, for a collaboration to maintain a shared project and use it instead incorporate the library, but I gave up because anyway KSNI is not what we want to use: it brings KWindowSystem as dependency (and probably other KDE stuff) in our projects, and 2 libraries are already too much for a QSystemTrayIcon replacement to achieve mouse wheel event handling.

EDIT: This should be the original post of both projects. @sebholt and I found some other links/resources about it while thinking how to manage the forks.

@redtide
Copy link

redtide commented Feb 27, 2024

After looking some of your work and this common idea to make a shared project with KDE people & co (and maybe being almost all Italians), I wonder if you mind to reach us on IRC or Matrix to have some random talk?
#lxqt on irc.oftc.net or #lxqt:matrix.org

@gfgit
Copy link
Member Author

gfgit commented Feb 27, 2024

@redtide Thanks for the invitation. I've joined the room on Matrix but my homeserver seems quite slow. I will have to change it at some point...

@redtide
Copy link

redtide commented Feb 27, 2024

it's a chat, no need for a fast connection there

@yan12125
Copy link
Member

yan12125 commented Mar 1, 2024

Here, we already have what we need. If KDE makes fixes that are usable to us, we could adapt them.

you can just ask them as you said

I have a similiar opinion like these. I'm not against using libraries from KDE, as long as it improves over the status quo.

  1. is a fork from this.

Just a note: it seems more like a fork from (2) than from this

@redtide
Copy link

redtide commented Mar 1, 2024

I'm not against using libraries from KDE, as long as it improves over the status quo.

The point is that probably they don't want to keep it alive, otherwise it wouldn't make sense for them to incorporate a part of it (menu exporter, removing the importer) in their KSNI. The version listed above (the first of the list) has the last commit from 2 years ago... though probably should be to take as reference because might have some other improvements too.

Just a note: it seems more like a fork from (2) than from this

@stefonarch we (@sebholt and I) haven't forked the LXQt version, because it's a fork of https://github.com/desktop-app/libdbusmenu-qt, which IIRC has some bugs; which is also the one used on Arch, the Qt6 version is broken, we forked it directly from https://github.com/unity8-team/libdbusmenu-qt and added some menu exporter improvements from the one incorporated in KSNI.

@Thaodan
Copy link

Thaodan commented Mar 14, 2024

I'm currently porting a few plasma plugins to Qt6.
One of them also depends on this library which right now uses an in-tree copy of the library similar to Plasma.
It would be great to not bundle the library.

Is the version of the lib in this repository working the same way as the original?

@redtide
Copy link

redtide commented Mar 14, 2024

I'm currently porting a few plasma plugins to Qt6. One of them also depends on this library which right now uses an in-tree copy of the library similar to Plasma. It would be great to not bundle the library.

Is the version of the lib in this repository working the same way as the original?

Depends on what you mean with "original", the original one was started more than 10 years ago by Aurélien Gâteau, at that time AFAIK KDE developer, then changed repositories over time. This is a fork of one of the variations of it. IIUC you are asking the same question me (somewhere else) and @gfgit did (here).
IIRC @tsujan was against to make it generic, while I also agree it should be DE agnostic and the same for all projects; so KDE should distribute it but apparently they are not interested and incorporate it instead inside KStatusNotifierItem (and IIUC also in something else you mentioned in your post).

For the reason above I created my (yet another) version, hoping someone else shares the idea since KDE people (at least it seems) are not with us. So I used the suffix "-qtilities" to not add further confusion, but it would be nice find enough people to move in a common place, use the original name and collaborate for maintenance.

@redtide
Copy link

redtide commented Mar 14, 2024

@tsujan: thinking of it: if this is used only in LXQt SNI, why don't you incorporate it like KSNI did?

For the menu issue I think it is working there (and so my version, which is a mix between lxqt qt plugin and KSNI), as I'm using it in my OMGMounter app, which has also a sni tray submenu.

@stefonarch
Copy link
Member

stefonarch commented Mar 14, 2024

I'm currently porting a few plasma plugins to Qt6.
It would be great to not bundle the library.

We have an issue with statusnotifier item menus not responding but are not yet sure what the cause is, as it worked fine in the Qt5-based statusnotifierplugin in lxqt-panel for Qt6 apps.

@tsujan
Copy link
Member

tsujan commented Mar 14, 2024

why don't you incorporate it like KSNI did?

It's used by more than one component, and it can be used outside LXQt.

@tsujan
Copy link
Member

tsujan commented Mar 14, 2024

while I also agree it should be DE agnostic

It is already. The "-lxqt" in its name doesn't mean it depends on LXQt, as the "K" in KWindowSystem doesn't mean that it depends on KDE.

@redtide
Copy link

redtide commented Mar 14, 2024

while I also agree it should be DE agnostic

It is already. The "-lxqt" in its name doesn't mean it depends on LXQt, as the "K" in KWindowSystem doesn't mean that it depends on KDE.

AFAICS usually when a qt version is implemented you remove previous version support, which is not something some people would like to discover at some point that their app stop working. So being in LXQt, who guarantee that support still apply instead doing what LXQt only requires?
IMO it should be in a neutral context with other people working.

In fact, I had hard time to use KSNI and KWindowSystem because not available for Qt5. In the case of the former also requiring a version of KWindowSystem too recent that even Arch didn't have, but added only recently.

@redtide
Copy link

redtide commented Mar 14, 2024

I'm currently porting a few plasma plugins to Qt6.
It would be great to not bundle the library.

We have an issue with statusnotifier item menus not responding but are not yet sure what the cause is, as it worked fine in the Qt5-based statusnotifierplugin in lxqt-panel for Qt6 apps.

I think because it was broken in the repo LXQt forked from, which also Arch uses and in fact libdbusmenu-qt6 in Arch is also broken (though they added their own patches, so might be by chance).

@tsujan
Copy link
Member

tsujan commented Mar 14, 2024

AFAICS usually when a qt version is implemented you remove previous version support,

When the main version of Qt is bumped (once in several years), it's natural to say goodbye to its older version when porting libraries to its new version. Everyone does it. But that "goodbye" doesn't make the older sources disappear from GitHub: they're among the released versions.

@tsujan
Copy link
Member

tsujan commented Mar 14, 2024

I think because it was broken in the repo LXQt forked from

We fixed that.

@redtide
Copy link

redtide commented Mar 14, 2024

I think because it was broken in the repo LXQt forked from

We fixed that.

We have an #9 with statusnotifier item menus not responding but are not yet sure what the cause is, as it worked fine in the Qt5-based statusnotifierplugin in lxqt-panel for Qt6 apps.

I'm talking about this, for the Qt6 changes

@tsujan
Copy link
Member

tsujan commented Mar 14, 2024

I'm talking about this, for the Qt6 changes

As @stefonarch said, we don't know its cause yet (I haven't started to look into it, being busy with other parts of LXQt).

@redtide
Copy link

redtide commented Mar 14, 2024

Yes, I was replying to him for the reason of the issue, which starts from the desktop-app fork commits and probably also with missing updates/fixes which was added meanwhile in KSNI version.
I don't recall the details, I faced this when starting with my own fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

6 participants