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.

Sign up for GitHub

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

Jump to bottom

Fix GotoDeskAndPage #837

Merged
merged 2 commits into from
Apr 2, 2023
Merged

Fix GotoDeskAndPage #837

merged 2 commits into from
Apr 2, 2023

Conversation

shang
Copy link
Contributor

@shang shang commented Mar 28, 2023

  • What does this PR do?
  1. Fix GotoDeskAndPage prev command
  2. Prevent duplicate new_desk events in GotoDeskAndPage

  • Screenshots (if applicable)

  • Issue number(s)
    2

@shang shang changed the title Shang/fix gotodeskandpage Fix GotoDeskAndPage Mar 28, 2023
ThomasAdam
ThomasAdam reviewed Mar 28, 2023
View reviewed changes
fvwm/virtual.c Outdated Show resolved Hide resolved
ThomasAdam
ThomasAdam previously requested changes Mar 28, 2023
View reviewed changes
Copy link
Member

@ThomasAdam ThomasAdam left a comment

Choose a reason for hiding this comment

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

Hi @shang

Thanks -- your previous version of this PR was definitely broken, so thanks for fixing that.

I am not sure this change is quite right -- can you confirm DesktopConfiguration shared isn't broken by this change?

@ThomasAdam ThomasAdam added the type:bug Something's broken! label Mar 28, 2023
@ThomasAdam ThomasAdam modified the milestones: 1.0.6a, 1.0.7 Mar 28, 2023
@ThomasAdam ThomasAdam self-assigned this Mar 28, 2023
@shang
Copy link
Contributor Author

shang commented Mar 29, 2023

Hi @ThomasAdam

I am using a laptop and has no extra monitors, I just changed DesktopConfiguration to shared and everything on my single monitor worked fine, but just as the log file said, "Shared mode only works if there is more than one monitor attached", so I believe my testing is insufficient, can you help test it if you are convenient?

Thanks~

shang added 2 commits April 2, 2023 21:11
@shang @ThomasAdam
GotoDeskAndPage: the prev argument never works
7d5d33f 
An incorrect value is assigned to prev_desk_and_page_desk, which makes
`GotoDeskAndPage prev` never working as expected.
@shang @ThomasAdam
GotoDeskAndPage: don't broadcast duplicate new_desk events
75bee53 
The new_desk event has already been broadcasted in CMD_GOTO_DESK.
@ThomasAdam ThomasAdam force-pushed the shang/fix-gotodeskandpage branch from 65b458b to 75bee53 Compare April 2, 2023 20:11
@ThomasAdam ThomasAdam dismissed their stale review April 2, 2023 20:12

Completed

@ThomasAdam
Copy link
Member

Hi @shang

Thank you for this PR. I've changed a few minor things, and will merge this now.

@ThomasAdam ThomasAdam merged commit 79e445d into fvwmorg:main Apr 2, 2023
@shang shang deleted the shang/fix-gotodeskandpage branch April 13, 2023 03:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ThomasAdam ThomasAdam ThomasAdam left review comments

Assignees

@ThomasAdam ThomasAdam

Labels
type:bug Something's broken!
Projects
FVWM3
Status: Done
Milestone
1.0.7
Development

Successfully merging this pull request may close these issues.
2 participants
@shang @ThomasAdam