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

Change default keybindings F1/F2 to F12/F11. #487

Merged
merged 1 commit into from
Jun 20, 2023
Merged

Change default keybindings F1/F2 to F12/F11. #487

merged 1 commit into from
Jun 20, 2023

Conversation

llc0930
Copy link
Contributor

@llc0930 llc0930 commented Mar 5, 2023

Just don't want an unexpected reboot when switching back to tty2.
Maybe you can consider change the default keybind.

@RannyArcher
Copy link
Contributor

i like that! i wish it be merged so soon.

@AnErrupTion AnErrupTion added the feature This issue or pull request discusses a feature label Jun 14, 2023
@AnErrupTion
Copy link
Collaborator

@lolicon0930 Hey! While we appreciate your contribution, I personally have a few issues with it:

  • You seem to have modified more files than necessary. It seems like those modifications are due to removing blank white spaces. While it's not that big of a deal, I still wonder why you did this exactly in this PR. (Unless it was automatic?)

  • I don't understand your issue of having an unexpected reboot when switching to TTY2. To switch to another TTY you have to press Ctrl + Alt + F2, not just F2, so I don't see the problem here.

  • Finally, to me it doesn't make sense to have F12 as shutdown and F11 as reboot, shouldn't it rather be the other way around? It would be more logical, at least in my opinion.

@llc0930
Copy link
Contributor Author

llc0930 commented Jun 16, 2023

@AnErrupTion, hope this is some reasonable explanation:

  • Yes, my editor settings include auto-removal of trailing whitespace, etc.
  • I don't need Ctrl when not in an X session, I just need Alt+F3 to switch from Ly's tty to tty3, and Alt+F2 to switch back to Ly's tty. The problem is, when I hold Alt and alternately press the function keys, there is a chance to reboot. I know Alt+Left/Right can also switch between ttys, but I don't usually use it that way.
  • For people like me who seldom shut down or use a computer as a server, F11 as shutdown and F12 as reboot are indeed more reasonable. But for the average person, when they turn on their computer and they're not logged in as a user, it's more likely that what they want to do in the display manager is turn off the computer.

@AnErrupTion
Copy link
Collaborator

I think a better solution would be to include an option in the configuration to change which function keys to use for shutdown and reboot. What do you think, @lolicon0930?

@llc0930
Copy link
Contributor Author

llc0930 commented Jun 20, 2023

@AnErrupTion Ok, no problem.

@llc0930
Copy link
Contributor Author

llc0930 commented Jun 20, 2023

Ah, right, I forgot to ask, is fg=9 the correct preset?

@AnErrupTion
Copy link
Collaborator

@lolicon0930 If you mean if it's the default value, then yes it is.

@llc0930
Copy link
Contributor Author

llc0930 commented Jun 20, 2023

Then why does the explanation say that ly only accepts values ​​from 0 to 8?

@AnErrupTion
Copy link
Collaborator

That's interesting, I never actually noticed this! It does only accept values from 0 to 8, however I have no idea when this little mistake slipped in 😅

@llc0930
Copy link
Contributor Author

llc0930 commented Jun 20, 2023

Start with an initial commit.

Copy link
Collaborator

@AnErrupTion AnErrupTion left a comment

Choose a reason for hiding this comment

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

That's the last change I'm requesting, otherwise it's good to go! Thanks for your contribution!

readme.md Outdated Show resolved Hide resolved
Include options in the configuration to change which function keys to use for shutdown and reboot.
Fix config.map_len size in src/config.c.
Add missing defaults in config_defaults() in src/config.c.
@AnErrupTion AnErrupTion merged commit 42bf929 into fairyglade:master Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This issue or pull request discusses a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants