Skip to content
This repository has been archived by the owner on Dec 3, 2021. It is now read-only.

Support 3.29.90 get neighbor workaround #81

Merged
merged 3 commits into from
Sep 4, 2018

Conversation

matttbe
Copy link
Contributor

@matttbe matttbe commented Aug 18, 2018

Hi,

This PR is based on the #80 with one more commit.

This is a workaround for GNOME 3.29.30 for this bug: https://gitlab.gnome.org/GNOME/mutter/issues/270
Without it, going left/right does nothing and down/up goes to the next/previous workspace. When you use to do that, it is disturbing :-)
Maybe other people also wants this workaround.

Even if it is a workaround, it should do exactly the same as before with older versions. It could then be merged as well and reverted later when the bug is fixed.

@@ -230,7 +232,7 @@ function indexToRowCol(index) {
*/
function rowColToIndex(row, col) {
// row-major. 0-based.
let idx = row * global.screen.workspace_grid.columns + col;
let idx = row * Utils.WS.getWS().workspace_grid.columns + col;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not create a constant for the Utils.WS.getWS() workspace manager?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This modification is inspired by what has been done in micheleg/dash-to-dock@8398d41
I suppose we cannot cache the workspace manager (even if I am not sure it can really change... maybe, I don't know :) )

@matttbe matttbe force-pushed the support-3.29.90-get_neighbor_workaround branch 2 times, most recently from 751f0f5 to fcd5e93 Compare September 1, 2018 19:38
@zakkak
Copy link
Owner

zakkak commented Sep 2, 2018

@matttbe thanks for your contributions, please have a look at my comments on how to potentially improve this PR

@matttbe
Copy link
Contributor Author

matttbe commented Sep 2, 2018

@zakkak thank you for your review! Please tell me what you prefer:

  • simplify the code by breaking compatibility as done in official GNOME Shell extensions
  • check the version (if it is possible but I guess yes)
  • check the signature / available variables as done in other extensions like micheleg/dash-to-dock@8398d41

@zakkak
Copy link
Owner

zakkak commented Sep 2, 2018

In general I would go for simpler code and sacrifice compatibility, however since 3.28 is the latest version for ubuntu 18.04 which is an LTS release and people are starting to move to 3.30 on other distros I guess compatibility is something we need.

On the other hand if we are going to share source code for different versions it would be better to have if statements that make clear which part of the if/else is for which version, that's why I prompted for checking the version instead of some method signature that changed in the latest release.

@matttbe
Copy link
Contributor Author

matttbe commented Sep 2, 2018

In general I would go for simpler code and sacrifice compatibility, however since 3.28 is the latest version for ubuntu 18.04 which is an LTS release and people are starting to move to 3.30 on other distros I guess compatibility is something we need.

You are right, I will keep the compatibility then!

On the other hand if we are going to share source code for different versions it would be better to have if statements that make clear which part of the if/else is for which version, that's why I prompted for checking the version instead of some method signature that changed in the latest release.

You are right, I will try to find something clearer.

Due to: https://gitlab.gnome.org/GNOME/gnome-shell/commit/47ea10b7
Inspired by micheleg/dash-to-dock@8398d41

- global.screen has been replaced by global.workspace_manager
- Meta.ScreenCorner has been replaced by Meta.DisplayCorner
- Main.wm._showWorkspaceSwitcher has less arguments (no more screen)

Signed-off-by: Matthieu Baerts <matttbe@gmail.com>
global.get_overrides_settings() is no longer available.

See: https://bugzilla.gnome.org/show_bug.cgi?id=786496

Signed-off-by: Matthieu Baerts <matttbe@gmail.com>
@matttbe matttbe force-pushed the support-3.29.90-get_neighbor_workaround branch from fcd5e93 to ef23483 Compare September 2, 2018 21:02
@zakkak zakkak merged commit 8cd1d2b into zakkak:3.28 Sep 4, 2018
@zakkak
Copy link
Owner

zakkak commented Sep 4, 2018

It turns out the PR breaks the extension on 3.28

image

@zakkak
Copy link
Owner

zakkak commented Sep 4, 2018

@matttbe please reopen the PR with edits that don't break 3.28 support or just make a totally incompatible with 3.28 version and I will merge it in a new branch

@matttbe
Copy link
Contributor Author

matttbe commented Sep 4, 2018

@zakkak thank you for having merge approved this.
I didn't verify if it was still supported in v3.28, I only checked by looking at the code.

Do you have more details about the error? I don't understand how Utils.WS.getWS(). Import are done in the two .js files I edited. I guess you have the new utils.js file in your extension directory.

I can try to check that on Gnome v3.28 but I am not sure I will have time before the week-end.

@zakkak
Copy link
Owner

zakkak commented Sep 4, 2018

I checked and the utils.js is indeed in the extension folder, not sure either why it breaks.

@matttbe
Copy link
Contributor Author

matttbe commented Sep 4, 2018

@zakkak by chance, do you have a line number? I suspect more a typo due to an option I don't use. I am not sure why the same function would not be defined in 3.28 :)

@zakkak
Copy link
Owner

zakkak commented Sep 4, 2018 via email

@matttbe
Copy link
Contributor Author

matttbe commented Sep 4, 2018

@zakkak I think you can use:

journalctl -f /usr/bin/gnome-shell -n 40

(note that the -f was not working for me recently, I didn't check why)

@zakkak
Copy link
Owner

zakkak commented Sep 4, 2018

Sep 05 02:35:25 slimbee gnome-shell[2696]: JS WARNING: [/home/zakkak/.local/share/gnome-shell/extensions/workspace-grid@mathematical.coffee.gmail.com/utils.js 32]: reference to undefined property "workspace_manager"
Sep 05 02:35:25 slimbee gnome-shell[2696]: Extension "workspace-grid@mathematical.coffee.gmail.com" had error: TypeError: Utils.WS.getWS(...) is undefined

@zakkak
Copy link
Owner

zakkak commented Sep 5, 2018

The issue appears to be related to how you calculate the ShellVersionId.

Please try something like the following instead:

const ShellVersionId = (parseInt(ShellVersion[0]) << 8) + parseInt(ShellVersion[1]);

@matttbe
Copy link
Contributor Author

matttbe commented Sep 5, 2018

Hi @zakkak,
Thank you, good catch :-)
Let me create a new PR with this fix!

@matttbe matttbe deleted the support-3.29.90-get_neighbor_workaround branch September 5, 2018 07:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants