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

windows.fnl: simply sequence of remove-monitor-items + add-monitor-items -> set-monitor-items #204

Merged
merged 4 commits into from
Jan 9, 2025

Conversation

jf
Copy link
Contributor

@jf jf commented Jan 8, 2025

remove-monitor-items and add-monitor-items are always run in sequence to basically set the monitor items. Using a single function both simplifies the code, and also makes it more understandable.

Copy link
Collaborator

@Grazfather Grazfather left a comment

Choose a reason for hiding this comment

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

Looks fine to me, but I am not familiar with this functionality.

windows.fnl Outdated Show resolved Hide resolved
windows.fnl Show resolved Hide resolved
@jf jf requested a review from Grazfather January 8, 2025 13:49
windows.fnl Outdated
@@ -483,15 +472,13 @@
Takes modal menu table-map
- Hides any previous display numbers
- Shows display numbers at top right of each screen
- Removes previous monitor items if any were added
- Adds monitor items based on currently connected monitors
- sets monitor items based on currently connected monitors
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit caps

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I think I know what you mean.. but I'm lost as to what "nit" means in this context. Would you mind explaining?

Copy link
Collaborator

Choose a reason for hiding this comment

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

means that I am nitpicking

Copy link
Collaborator

Choose a reason for hiding this comment

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

As in the PR is fine, but I am trying to fix tiny things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it. Thank you!

@jf jf requested a review from Grazfather January 9, 2025 02:44
@Grazfather
Copy link
Collaborator

Thanks for your contribution!

@Grazfather Grazfather merged commit ecd66b0 into agzam:master Jan 9, 2025
@jf
Copy link
Contributor Author

jf commented Jan 9, 2025

Thanks for your contribution!

and thank you for taking the time to review and look through things!

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.

2 participants