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

Cleanup FvwmPager's window handling methods. #994

Merged
merged 4 commits into from
Apr 4, 2024
Merged

Conversation

somiaj
Copy link
Collaborator

@somiaj somiaj commented Apr 4, 2024

Remove a lot of duplicate code around the creation of, the moving of,
hiding of, and even the destruction of the mini XWindows FvwmPager
uses to represent the real windows.

  • Remove ReConfigureIcon(), the method MoveResizePagerView() in ReConfigureAll() was already updating the icon windows. No need for a specialized function.
  • Remove all the logic from the pager view's approach of creating and destroying windows as they came in and out of view of the pager. Instead do the same thing as the icon view, just move all the windows out of view. Desk[0] is used to stash all windows that don't have a home otherwise. Now it can be assumed that the windows always exist and no longer a need to check for that.
  • Take all the methods that would use CalcGeom() to update the windows, and combine all the code into single methods. CalcGeom() is now called in a single location, and the creation of, and managing of all windows is uniform.

This work so far so good in my tests, managing behavior of the windows has become exponentially easier.

@somiaj somiaj requested a review from ThomasAdam April 4, 2024 07:20
@somiaj somiaj force-pushed the js/pager-cleanup branch 5 times, most recently from b4474f7 to d5f4581 Compare April 4, 2024 19:07
@ThomasAdam ThomasAdam self-assigned this Apr 4, 2024
@ThomasAdam ThomasAdam added relates:module Issue is in module code type:bug Something's broken! labels Apr 4, 2024
@ThomasAdam ThomasAdam added this to the 1.1.1 milestone Apr 4, 2024
somiaj added 4 commits April 4, 2024 20:18
  Remove a lot of duplicate code around the creation of, the moving of,
  hiding of, and even the destruction of the mini XWindows FvwmPager
  uses to represent the real windows.

  * Remove ReConfigureIcon(), the method MoveResizePagerView() in
    ReConfigureAll() was already updating the icon windows. No need
    for a specialized function.
  * Remove all the logic from the pager view's approach of creating
    and destroying windows as they came in and out of view of the pager.
    Instead do the same thing as the icon view, just move all the windows
    out of view. Desk[0] is used to stash all windows that don't have
    a home otherwise. Now it can be assumed that the windows always
    exist and no longer a need to check for that.
  * Take all the methods that would use CalcGeom() to update the windows,
    and combine all the code into single methods. CalcGeom() is now called
    in a single location, and the creation of, and managing of all windows
    is uniform.
  The showing MiniSize windows snapping code had an extra `+ 1`, which
  made the windows border not show as it was one pixel off the pager.
  This fixes that issue.
The Bool type is defined by xlib as a historical type, which is just an
int.  However, this should only be used for Xlib syscalls, and other
bool types can now use "bool" from stdbool.h
  Move all shared variables to FvwmPager.h, and label static
  variables as such. Also group the various variable types together.
@ThomasAdam ThomasAdam force-pushed the js/pager-cleanup branch 2 times, most recently from 2fe335e to 4d81aaa Compare April 4, 2024 19:20
@ThomasAdam ThomasAdam merged commit 881c192 into main Apr 4, 2024
5 checks passed
@ThomasAdam ThomasAdam deleted the js/pager-cleanup branch April 4, 2024 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relates:module Issue is in module code type:bug Something's broken!
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants