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

Fix focus.global_bydirection not moving focus from empty screen #3815

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JaCoB1123
Copy link

When having three monitors and only one client on the first monitor, the function does not work when the middle monitor is focused, as it uses the focused clients' screen instead of the currently focused screen as the base.

When having three monitors and only one client on the first monitor, the function does not work when the middle monitor is focused, as it uses the focused clients' screen instead of the currently focused screen as the base.
@codecov
Copy link

codecov bot commented May 21, 2023

Codecov Report

Merging #3815 (a9136bc) into master (b13ac3e) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3815      +/-   ##
==========================================
+ Coverage   90.99%   91.02%   +0.02%     
==========================================
  Files         900      901       +1     
  Lines       57506    57530      +24     
==========================================
+ Hits        52329    52365      +36     
+ Misses       5177     5165      -12     
Flag Coverage Δ
gcov 91.02% <100.00%> (+0.02%) ⬆️
luacov 93.71% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
lib/awful/client/focus.lua 92.04% <100.00%> (+0.18%) ⬆️
tests/test-focus-bydirection.lua 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

@actionless
Copy link
Member

@JaCoB1123
Copy link
Author

JaCoB1123 commented May 22, 2023

The failing assertion seems to concern awful.client.focus.bydirection("right") which I did not modify. No idea how that happened? Also seems to only be the case for lua5.1? Any hints, what I should do?

@actionless
Copy link
Member

@sclu1034
Copy link
Contributor

sclu1034 commented May 22, 2023

Any hints, what I should do?

The first step would be to run the tests locally. make check, IIRC.

@JaCoB1123
Copy link
Author

JaCoB1123 commented May 24, 2023

Ok, didn't see that. That error is even weirder though, as it's in line 21 even before calling any of the focus functions.

Any hints, what I should do?

The first step would be to run the tests locally. make check, IIRC.

Thanks for the hint. I unfortunately I have never used lua aside from my awesome config. Therefore I haven't got a local lua dev environment and wasn't able to set one up either because of 'lgi' being missing. I even tried installing lgi by using the steps from the github actions file.

I can't invest more time here, so feel free to close if nobody else is willing to take over.

@Aire-One
Copy link
Member

Given this other PR #3817 (that only changes unrelated documentation) has a similar CI fail (the fail is on the same test at least), I guess this is rather an instability in our CI.

@actionless
Copy link
Member

indeed it weirdly failed for both of them at the same time - but restarting helped for both

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.

4 participants