Jazqa / kwin-quarter-tiling

An easy tiling script for KWin
GNU General Public License v2.0
367 stars 20 forks source link

Cannot vary gap in typescript-rework branch #81

Closed kupiqu closed 4 years ago

kupiqu commented 4 years ago

It always considers a gap of 8 (which is the default) independently of varying its value anywhere else (in the UI or in kwinrc).

The only workaround I found is modifying its value in main.js line 51:

var gaps = readConfig("gaps", 2);
Jazqa commented 4 years ago

Thanks for reporting. It’s most likely a minor typo, I’ll fix it as soon as I get on a computer.

Jazqa commented 4 years ago

Should be fixed with the latest commit.

kupiqu commented 4 years ago

Great, it works.

Unfortunately, however, the apply button is disabled and to update the change I needed to restart kwin.

kupiqu commented 4 years ago

By the way, unrelated to this, but also affecting this branch: to filter out latte-dock (e.g., opening the systray in latte), I needed to add "Latte Dock" to the list of Ignored Programs.

Jazqa commented 4 years ago

Thanks for reporting! I might have to add "Latte Dock" to the default list of Ignored Programs, as it seems to pass the window category check that makes the script ignore most special windows. Still curious why it's ignored with the old script though, as the list of ignored windows and programs is the same.

kupiqu commented 4 years ago

Yes, that's what I did, I added Latte Dock to the list, and that fixed the issue. No clue why the other options sufficed before but not anymore.

Any clue about the other issue mentioned above relative to the Apply button?

kupiqu commented 4 years ago

Also, is there a reason why not to set the minimum possible gap to 1? I think I prefer that option

Jazqa commented 4 years ago

For the Apply button, I have no idea. I'll have to look into that. Was the whole Apply button disabled or pressing it just had no effect? If I remember correctly, both versions of the script require enabling/disabling the script for the configuration changes to apply.

For gaps, I think I can do a minimum value of 1. Unfortunately 0 will not be possible, because the script wouldn't be able to identify maximized windows.

kupiqu commented 4 years ago

The apply button is disabled (grayed out) so clicking on it is not possible. I guess it is because the change is not communicated or sth.

kupiqu commented 4 years ago

For the case I was not clear earlier, I mean the Apply button in Kwin Scripts, not the Ok button inside the config UI of Quarter Tiling (this latter one is clickable, and one would expect that it enables the former one, so if the Apply buttons is clicked, the setting is applied to open windows).

Or perhaps this Apply button is for changes when enabling/disabling kwin scripts and it's just fine for it to be greyed out if no such change has been made. If so, the issue is in the Ok button inside Quarter Tiling settings. It seems that the OK button properly applies the setting in kwinrc, but it fails to apply that change to current clients.

EDIT: I just tested this, and actually a temporary workaround is the following:

  1. Change any setting in Quarter Tiling > Click Ok to apply it
  2. It fails to apply the change on-the-fly (to current and new clients)
  3. Disable Quarter Tiling
  4. Click Apply
  5. Enable Quarter Tiling
  6. Click Apply
  7. The change in (1) is applied on-the-fly (to current and new clients)
kupiqu commented 4 years ago

Two more issues I identified in this branch:

  1. Most shorcuts don't seem to work, in particular, the reset shortcut that I use all the time
  2. New windows appear on top of maximized windows (I actually like this better than moving to a different desktop, which was the previous default, as one can move those manually if wanted with a kwin shortcut if wanted). However, maximized windows are not considered to limit the number of clients up to 4, and then restoring those windows breaks tiling.

Also, the script doesn't seem to be as compatible with the Sticky Windows Snapping Script as before, or perhaps I fixed these minor issues on the fly before with the reset shortcut mentioned above (1)

Jazqa commented 4 years ago

Once again, thanks for helping out! These reports really help, as I'm not using my personal KDE setup as much as I'd want to.

Most shorcuts don't seem to work, in particular, the reset shortcut that I use all the time

No worries! Shortcuts are a work-in-progress. Many missing shortcuts (including reset) will deifnitely be there soon.

New windows appear on top of maximized windows (I actually like this better than moving to a different desktop, which was the previous default, as one can move those manually if wanted with a kwin shortcut if wanted). However, maximized windows are not considered to limit the number of clients up to 4, and then restoring those windows breaks tiling.

One of the largest problems, code-wise and usability-wise, is the sheer amount of tiny features such as the one you mentioned (suddenly moving to a desktop when the current one has a maximized window). They were all created to fill a need I had back then, but I've grown to dislike some of them and I bet many users have as well. That's why my main goal currently is to create a simple and robust script to replace the current master and add usability options in later.

Maximized/minimized windows are problematic, so for the first release I'm using the easiest approach, which is removing maximied/minimized windows and adding them as "new" windows when they are unmaximized/unminimized (essentially the same behavior as floating/unfloating). Unmaximizing/unminimizing should not break the tiling though, so fixing that is going to be a high-priority task for me.

Counting maximized/minimized windows towards the tiling limit and reserving their spot is totally doable, but that requires quite a bit of extra logic, which is why I consider that as one of those extra usability features I'll add once the rework is merged into master and the script is stable.

kupiqu commented 4 years ago

Sounds great, thank you!

Let me be a more specific on what I mean by restoring those windows breaks tiling:

This only affects the case when restoring a window from maximization when there are already 4 tiled clients. And what happens is that the restored windows goes on top of one of these other clients occupying a quarter (5 clients distributed in 4 quarters). However, the restored client is only apparently tiled, and actually floating, which is clearly seen by moving the window. The 4 clients that were originally tiled still are, so tiling for them is not broken.

I guess the immediate fix when restoring a maximized window that surpasses the limit of N=4, is moving that window to a different desktop. It is a bit counterintuitive perhaps, but I think it's a good move until the extra logic of considering them in setting the limit is added.

kupiqu commented 4 years ago

In addition to Latte Dock you may want to add ksmserver-logout-greeter to the exclude list to avoid tiling the logout dashboard.

Jazqa commented 4 years ago

Thanks for the ksmserver-logout-greeter! That was definitely an issue.

As for the "Apply" button, I may not be able to do anything about it. From what I've understood, the "Apply" button is meant for changes in the script selection, so changes to settings of a specific script will not enable it. I may, however, be able to make the script restart when the configuration is changed - I'll have to look into it and see if I can get it working.

kupiqu commented 4 years ago

Yes, I think you are right. This is in line with what I wrote above:

It seems that the OK button properly applies the setting in kwinrc, but it fails to apply that change to current clients.

EDIT: I just tested this, and actually a temporary workaround is the following:

  1. Change any setting in Quarter Tiling > Click Ok to apply it
  2. It fails to apply the change on-the-fly (to current and new clients)
  3. Disable Quarter Tiling
  4. Click Apply
  5. Enable Quarter Tiling
  6. Click Apply
  7. The change in (1) is applied on-the-fly (to current and new clients)
Jazqa commented 4 years ago

Yep. Fortunately KWin scripting API has a configChanged signal. If I start using that, I'll just make the whole script restart instead of applying the specific changes, because changes like changing ignored activities/desktops etc. on the fly would require quite a bit of extra logic.

Also, minimum gap is now 1. If I had a reason to set it as 2, I can't remember it.

I guess the immediate fix when restoring a maximized window that surpasses the limit of N=4, is moving that window to a different desktop. It is a bit counterintuitive perhaps, but I think it's a good move until the extra logic of considering them in setting the limit is added.

Yeah, technically it should work like this now - there's just some minor bug I'll have to figure out to fix it. I'll most likely be working on the script for some hours during the weekend, so there should be at least some improvements on the way!

kupiqu commented 4 years ago

Sounds great!

Jazqa commented 4 years ago

The unmaximize and unminimize should work now.

kupiqu commented 4 years ago

The unmaximize and unminimize should work now.

Awesome!

Jazqa commented 4 years ago

I added the restore shortcut (for the current screen/desktop and for all the screens/desktops). However, shortcuts will conflict with the old version of the script. Unfortunately KWin never clears the kglobalshortcutsrc file, so the shortcuts of the old script will remain in the shortcuts list. One way to easily get rid of this problem is to locate the file (for me it's ~/.config/kglobalshortcutsrc) and clear all the Quarter: entries.

kupiqu commented 4 years ago

Great!

One way to easily get rid of this problem is to locate the file (for me it's ~/.config/kglobalshortcutsrc) and clear all the Quarter: entries.

I think one needs to kill kwin before making those changes. Alternatively one can just go to the shortcuts settings and redirect the reset shortcut (I don't think I will come back to the previous version, but prefer keeping those other shortcuts for the moment). I did this and works!

Now I could revisit this:

the script doesn't seem to be as compatible with the Sticky Windows Snapping Script as before

The prev version of quarter tiling allowed higher priority for snapping over quarter tiling (which I think is the desired behavior for users of snapping). Now this works as expected when resizing horizontally, but resizing vertically breaks snapping, so the window that is not being directly resized is reset to quarter tiling.

I hope you understand what I mean, and hopefully there is an easy fix for recovering compatibility with snapping...

kupiqu commented 4 years ago

There is, I think, an important issue with Automatically change desktops: when there is only one client and that client is then maximized or set full screen (e.g. video), the virtual desktop changes to another one, even though the (maximized/full screen) client stays in the original desktop. This is quite confusing as the user does not know what is going on, i.e., what happened to the client.

For the time being I workaround the issue disabling the setting: Automatically change desktops

EDIT: Other than that,Automatically change desktops works just lovely. I hope I can enable it as soon as possible :)

Jazqa commented 4 years ago

Good find, I'll fix that tonight!

Jazqa commented 4 years ago

Should work now. Minimizing the last client still changes desktop when it's the last window on the desktop (makes sense, since you'd have an empty desktop otherwise).

kupiqu commented 4 years ago

Excellent, as usual!

Jazqa commented 4 years ago

I hope you understand what I mean, and hopefully there is an easy fix for recovering compatibility with snapping...

I'm sorry I don't. I have snapping enabled and horizontal and vertical both feel the same.

kupiqu commented 4 years ago

ok, I'll try to make a couple of gifs to illustrate the issue with snapping...

In the time being, another thing that would be nice improving is making these fields adapt to the available window size, not sure though if this is technically challenging:

image

kupiqu commented 4 years ago

Oh, I realized that snapping comes from the quarter tiling script not from the sticky window snapping because they actually don't touch each other

Snapping from quarter tiling in my system only works when resizing windows horizontally:

image

but not vertically:

image

Jazqa commented 4 years ago

Alright, I think I understand.

The only layout currently available works like this:

                       Separator 1
                           |
                           v
                         1 | 2
         Separator 2 ->  --|-- <- Separator 3
                         4 | 3

And you'd prefer a layout like this:

                       Separator 1
                           |
                           v
                         1 | 2
         Separator 2 ->  --|--
                         4 | 3
kupiqu commented 4 years ago

Oh, I see the logic now.

Perhaps it's just fine as it is, but you may also consider a snapping setting: none, horizontal, vertical, all?

none would resize each window on its own adapting only neighbor quarters, horizontal would be current behavior, vertical would be the orthogonal behavior, and all would be the behavior such as the one in sitcky windows snapping

Would that make sense?

Naively, I guess this only makes sense if technically this is heuristics that is not complex to implement...

Jazqa commented 4 years ago

Yes. I’ve though about it before and I will do something like it. However, they’ll appear as different ”Quarter” layouts instead of a ”snapping” setting.

Also, the ”none” in your suggestion would not work when tiling four windows to quarters – there has to be at least one ”major” separator splitting the screen either horizontally or vertically.

kupiqu commented 4 years ago

Oh, yeah, that's right: horiz || vert || both

kupiqu commented 4 years ago

I am using click to focus in Kwin and I think clicking on the maximization button should also automatically gain focus if the window didn't have it. Perhaps it's just me but I find the default Kwin behavior quite counterintuitive maximizing without gaining focus.

I guess I should report this to Kwin, but for the time being, do you think this suggestion would make sense in the context of quarter tiling? Would quarter tiling be able to give focus to maximized clients so the previous active quarter client does not appear above?

EDIT – I submitted the issue to Kwin here: https://bugs.kde.org/show_bug.cgi?id=418938

Jazqa commented 4 years ago

Three layouts available (Quarter V, Quarter H and Quarter Both) - testing for them, however, was not very thorough. Also, technically I’d still need to add another ”Quarter Both” that imitates the vertical tiling.

Configuration UI slightly improved (not by much, making it truly scalable is too painful - I'm not a fan of the qt5designer).

Maximizing a client now focuses it. That was a no-brainer, I don't see any reason to not focus the maximized client.

kupiqu commented 4 years ago

Excellent!

kupiqu commented 4 years ago

Three layouts available (Quarter V, Quarter H and Quarter Both) - testing for them, however, was not very thorough. Also, technically I’d still need to add another ”Quarter Both” that imitates the vertical tiling.

The new changes in layouts don't seem to work on my end, meaning tiling stopped working and had to went few commits back to get it back. I guess you're still testing them :)

kupiqu commented 4 years ago

The new changes in layouts don't seem to work on my end, meaning tiling stopped working and had to went few commits back to get it back. I guess you're still testing them :)

It may actually be that the issue is in commit e5b46952c2d947783fe734080e8e686a9cf0159d as I need to go back to the previous to that one

kupiqu commented 4 years ago

Found the issue: tiling is disabled if the content of ignored captions is empty

I think it's related to this:

@@ -106,7 +106,7 @@ function includes(client) {
         client.screen < 0 ||
         client.geometry.width < config.minWidth ||
         client.geometry.height < config.minHeight ||
-        config.ignoredCaptions.indexOf(client.caption.toString()) > -1 ||
+        config.ignoredCaptions.some(function (caption) { return client.caption.toString().indexOf(caption) > -1; }) ||
         config.ignoredClients.indexOf(client.resourceClass.toString()) > -1 ||
         config.ignoredClients.indexOf(client.resourceName.toString()) > -1 ||
         config.isIgnoredDesktop(client.desktop) ||

EDIT: As a workaround I just added test to the list of ignored captions so that is not empty and tiling came back and working nicely in latest version of the branch

kupiqu commented 4 years ago

A very minor thing is the different font size of Automatically tile windows in settings:

image

Jazqa commented 4 years ago

Oh yeah. It's matching substring for captions - so ignoring caption "Spotify" ignores all windows with "Spotify" in the caption. A little mistake there... when ignored captions are empty it matches all windows containing "", which is all of them.

kupiqu commented 4 years ago

Oh yeah. It's matching substring for captions - so ignoring caption "Spotify" ignores all windows with "Spotify" in the caption. A little mistake there... when ignored captions are empty it matches all windows containing "", which is all of them.

that's actually great. could you make it not sensitive to case?

Jazqa commented 4 years ago

that's actually great. could you make it not sensitive to case?

Fixed the bug and made it case insensitive. Also fixed the font inconsistencies in the UI.

kupiqu commented 4 years ago

that's actually great. could you make it not sensitive to case?

Fixed the bug and made it case insensitive. Also fixed the font inconsistencies in the UI.

Great! I'm closing this issue as everything works like a charm

kupiqu commented 4 years ago

One additional issue I found is when resizing a maximized window (if allowed via borders or grip) breaks tiling for that client. Could you please try to fix this?

Jazqa commented 4 years ago

One additional issue I found is when resizing a maximized window (if allowed via broders or grip) breaks tiling for that client. Could you please try to fix this?

Seems like KWin doesn't send the signal for unmaximizing a client if it's unmaximized via resizing. It's not a very simple fix, so it'll have to wait for me to implement improved support for maximized clients (e.g. having a maximized client subtract from the maximum tiled clients on that desktop).

kupiqu commented 4 years ago

I didn't know this is an upstream issue. Given the case, I'd suggest not to work around the issue if that requires a dirty hack.

I will fill a kwin bug in kde, maybe kde devs are receptive to this.

Jazqa commented 4 years ago

I'll have to investigate it a little more to be 100% certain, but from what I quickly checked, the signal is not fired.

The workaround is not much of a hack, and I'm going to have to do it anyways when implementing features like having a maximized client subtract from the maximum tiled clients on that desktop and returning a maximized client to the spot it was maximized from.

kupiqu commented 4 years ago

KDE bug: https://bugs.kde.org/show_bug.cgi?id=419193

kupiqu commented 4 years ago

Two further minor issues:

image