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

Combined KDE 5 and KDE 6 compatibility #8

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

RedBearAK
Copy link

@RedBearAK RedBearAK commented Nov 23, 2023

@nclarius

This branch has code that is now compatible with both KDE 5 and KDE 6. It uses an API abstraction strategy to use the relevant API references for the KDE major version, at runtime.

The installer and uninstaller Bash scripts have also been reworked to intelligently check for the major KDE release version and use the appropriate version of the command line tools, and offer useful error messages if something goes wrong.

Potential lack of availability of the qdbus utility (such as on KDE Neon, which has gdbus instead), is handled gracefully by checking for multiple D-Bus utilities and using whatever is available to perform the KWin "reconfigure" action.

This PR also includes the important grouping fixes from PR #7, that fixed some problems that were causing loss of the app grouping due to undefined window errors from things like the task switcher dialog.

Bumped version in READMEs and metadata file to 1.7.

Merging this should close issue #6.

@RedBearAK
Copy link
Author

@nclarius

It's been a couple of weeks, so I'm prodding you (as requested) to take a look at this PR.

I've been installing directly from my own branch based on this PR for months now, on both Plasma 5 and Plasma 6, in multiple distros, every time I test my project (Toshy) in a KDE environment. Haven't found any further issues so far.

The error conditions that affect the currently published version of the script make it very unreliable at grouping app windows together. All of that is fixed in this PR.

Merging this should close both issue #6 (grouping stops working) and issue #9 (no support for Plasma 6).

@cjbottaro
Copy link

I cloned your repo and switched to the kde6_kde5_merged branch and the install.sh failed for me:

./install.sh 
Installing KWin script: 'applicationswitcher'
KPackageStructure of KPluginMetaData(pluginId:"applicationswitcher", fileName: "/home/cjbottaro/.local/share/kwin/scripts/applicationswitcher/metadata.json") does not match requested format "KWin/Script"
Error: Plugin applicationswitcher is not installed.

ERROR: Problem installing 'applicationswitcher' with kpackagetool6. 

I also tried to click the "Install from file..." button, but this PR has removed application-switcher_v1.6.kwinscript file, so there is nothing to select.

Any suggestions?

@RedBearAK
Copy link
Author

@cjbottaro

Not sure. The metadata.json you have there should have this line:

"KPackageStructure": "KWin/Script",

You can try editing this function to remove the redirect to &> /dev/null, to see more clearly what's going wrong when the command runs:

install_w_kpackagetool6() {
    if ! command -v kpackagetool6 &> /dev/null; then
        exit_w_error "The 'kpackagetool6' command was not found. Cannot install KWin script."
    fi
    echo "Installing KWin script: '${script_name}'"
    if ! kpackagetool6 --type="${script_type}" --install "${script_path}" &> /dev/null; then
        kpackagetool6 --type="${script_type}" --upgrade "${script_path}"
    fi
}

What is your distro?

@RedBearAK
Copy link
Author

@cjbottaro

I just added a .kwinscript file to the branch, for v1.7, according to instructions from GPT-4. It said the format should be just a zip file with main.js and metadata.json inside, with no additional folder structure. But the v1.6 .kwinscript file seems to contain all files in the repo, with the existing folder structure. So I'm not sure what I did was the correct way to do it.

To see the file you'll probably need to do a git pull or git fetch in the cloned folder.

@RedBearAK
Copy link
Author

@cjbottaro

Here's a version of the .kwinscript file made like the old one, with all files and folders in the repo. Just remove the ".zip" from the file name to try it. Had to leave it with the zip extension to attach it here.

application-switcher_v1.7.kwinscript.zip

@RedBearAK
Copy link
Author

@cjbottaro

I think your problem is that your terminal or file manager is not seeing the correct state of the files after you used git to switch branches. I've seen this before, which is why when I install something from GitHub from a script with a git clone, I set up the script to first destroy any existing local folder with the same name as the repo I'm cloning. Just to avoid trouble and have assurance that the state of the cloned repo I end up installing from is actually the latest state of that branch. In short, git has some strange behavior sometimes that seems to confuse terminals and file managers.

I would recommend just downloading the zip file (instead of using git) after switching to the kde6_kde5_merged branch on my fork of this repo (or just click the link to my branch shown under the title of this pull request), and unzip the file and install from there.

I just did a fresh, unmodified and updated install of KDE Neon User in a virtual machine, and had no error when running the ./install.sh script. The error you ran into really suggests to me that you were somehow installing a version of the KWin script that still had a metadata.json file that wouldn't work with the Plasma 6 tools. The one that was missing the line I showed earlier.

Remember to check the "Only one window per application" option and use "Large Icons" in Task Switcher settings if you want this to appear to work exactly like macOS or GNOME's "Switch applications". Otherwise there will be an icon or preview for each window of any application that has multiple windows open on the same desktop, and the switching will be a bit strange, although the grouping does still work in that case. It's just more confusing than it should be if you don't pick those Task Switcher options.

@cjbottaro
Copy link

Ahh, I see what you mean. Ok, so I tried downloading your zip, removing the .zip extension, and installing via the UI.

I get this error:
image

The thing is... I don't see any old version installed in the UI. Also running the uninstall.sh script fails with:

./uninstall.sh 
Removing 'applicationswitcher' KWin script.
kf.package: Could not find required file "mainscript" for package "/home/cjbottaro/.local/share/kwin/scripts/" should be QList("code/main.js")
kf.package: Could not find required file "mainscript" for package "/usr/share/kwin/scripts/" should be QList("code/main.js")
KPackageStructure of KPluginMetaData(pluginId:"applicationswitcher", fileName: "/home/cjbottaro/.local/share/kwin/scripts/applicationswitcher/metadata.json") does not match requested format "KWin/Script"
Error: Plugin  is not installed.

ERROR: Problem while removing 'applicationswitcher' KWin script. 
Exiting...

Granted I'm using the uninstall.sh from your version, but I don't remember where/how I installed the other version. It doesn't help that it doesn't show up in the UI either. :(

@RedBearAK
Copy link
Author

RedBearAK commented May 12, 2024

@cjbottaro

OK, looks like the first suggestion from GPT-4 was completely wrong. I can't really blame it because I've found absolutely no meaningful reference on the web about how to make a .kwinscript file. But now I made one that just includes all the files in the repo, with the existing folder structure for the KWin script components. Should work just like the v1.6 one did.

First, go to /home/cjbottaro/.local/share/kwin/scripts/ in the terminal or a file manager and manually delete any applicationswitcher folder that exists there. Then grab a new zip again and try using the new version of the .kwinscript file.

If it doesn't work again, I suspect you might still be running into the error complaining about the script type, but since you haven't specified your Linux distro I have no way to attempt to replicate the problem. I frequently install this KWin script alongside another project of mine in different virtual machines with different Linux distros where the desktop environment is Plasma 5 or 6, and I haven't run into this issue.

If you can say more about your system, it would be helpful. Distro, distro version, Plasma version, etc.

@RedBearAK
Copy link
Author

RedBearAK commented May 12, 2024

@cjbottaro

I can replicate the uninstall script error in Fedora 40 with Plasma 6, but the install script and installing from the kwinscript file have no issue. I think this is separate from the original install error.

In an openSUSE Leap VM with Plasma 5, all methods of installing and uninstalling work as expected.

The problem with uninstalling makes no sense as I used the same tool kpackagetool6 to do the install, which works perfectly. Then the uninstall option claims it can't find the required file. The manual commands are actually quite simple:

kpackagetool6 --type=KWin/Script --install .
kpackagetool6 --type=KWin/Script --remove .

That's if you are inside the folder that came out of the zip file downloaded from GitHub.

EDIT: As noted below in the next comment, the second command given above is actually using the wrong syntax. It should be passing the script name instead of a path, and I modified the uninstall script to get rid of those "mainscript" errors. 🤦🏽

kpackagetool6 --type=KWin/Script --remove "applicationswitcher"

@RedBearAK
Copy link
Author

@cjbottaro

I was giving the remove command a path instead of the script name. Apparently that was always wrong, even though I never saw those error messages before. I modified the uninstall script to use the correct syntax and pass the script name when removing the script.

This doesn't explain or get rid of the script type error you've been running into, which I still don't know how to replicate without knowing more about your environment. Please get back to me with more information when you can.

@RedBearAK
Copy link
Author

I updated the uninstall script to properly unload the KWin script from memory before removing the files, using any available D-Bus utility. Tested in Plasma 5 and 6. Working well so far.

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

2 participants