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: fix infinte loop when prompting for SDDM themes #41

Merged
merged 1 commit into from
Mar 7, 2024
Merged

fix: fix infinte loop when prompting for SDDM themes #41

merged 1 commit into from
Mar 7, 2024

Conversation

cbaconnier
Copy link
Contributor

Pull Request

Description

As I was running the script on my VM, I hit x instead of y and got stuck in an infinite loop.
Resetting the prompt variable (install_sddm_theme) fix the issue.
I haven't checked if the issue was present elsewhere in the scripts.

Type of change

Please put an x in the boxes that apply:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update (non-breaking change; modified files are limited to the documentations)
  • Technical debt (a code change that does not fix a bug or add a feature but makes something clearer for devs)
  • Other (provide details below)

Checklist

Please put an x in the boxes that apply:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My commit message follows the commit guidelines.
  • My change requires a change to the documentation.
  • I want to add something in Arch-Hyprland wiki.
  • I have added tests to cover my changes.
  • I have tested my code locally and it works as expected.
  • All new and existing tests passed.

Additional context

Tested locally with a minimal reproducible example

# SDDM-themes
valid_input=false
while [ "$valid_input" != true ]; do
  if [[ -z $install_sddm_theme ]]; then
    read -n 1 -r -p "OPTIONAL - Would you like to install SDDM themes? (y/n)" install_sddm_theme
  fi
  if [[ $install_sddm_theme =~ ^[Yy]$ ]]; then
    printf "\n%s - Installing Simple SDDM Theme\n"
    valid_input=true
  elif [[ $install_sddm_theme =~ ^[Nn]$ ]]; then
    printf "\n%s - No SDDM themes will be installed.\n"
    valid_input=true
  else
    printf "\n%s - Invalid input. Please enter 'y' for Yes or 'n' for No.\n"
    # uncomment for the fix
    # install_sddm_theme=""
  fi
done

@JaKooLit
Copy link
Owner

JaKooLit commented Mar 7, 2024

its written there.... type y or n NOT x

and yes it will loop since it will only accept either Y y or N n

@JaKooLit JaKooLit closed this Mar 7, 2024
@cbaconnier
Copy link
Contributor Author

cbaconnier commented Mar 7, 2024

@JaKooLit I agree, it should only accept Yy/Nn but we shouldn't be stuck in a infinite loop. Currently the only way is to type Ctrl+c and quit the script.

I pressed x because a mistyped. (On my keyboard it next to y)

Even better, if [[ -z $install_sddm_theme ]]; then could be removed, resetting the variable shouldn't be necessary afterward.

Peek.2024-03-07.21-15.mp4

@JaKooLit
Copy link
Owner

JaKooLit commented Mar 7, 2024

what happen if you press n?

@cbaconnier
Copy link
Contributor Author

cbaconnier commented Mar 7, 2024

what happen if you press n?

Nothing, it keep going

@JaKooLit
Copy link
Owner

JaKooLit commented Mar 7, 2024

hmmmm weird....

I just tried now, choosing n will just skip installing sddm

can you attach here the sddm log?

@cbaconnier
Copy link
Contributor Author

cbaconnier commented Mar 7, 2024

Sure!

To be sure we are on the same page:

what happen if you press n?

  • If I press n, It will correctly show No SDDM themes will be installed
  • If I press y, It will correctly install the SDDM (well at least, I know it will pass the if condition)
  • But if I press x, I won't get any chance to press n after and I will just be stuck in a loop forever.

--

After the first loop : install_sddm_theme will still contains x so this condition if [[ -z $install_sddm_theme ]]; will be false (since the variable is not empty) and we won't have a chance to be re-prompting OPTIONAL - Would you like to install SDDM themes? (y/n) to the user.

Therefore, install_sddm_theme will never change and we will go down to the last else condition, resulting in a loop.

install-07-202805_sddm.log

@JaKooLit JaKooLit reopened this Mar 7, 2024
@JaKooLit JaKooLit merged commit 80dad5f into JaKooLit:main Mar 7, 2024
@JaKooLit
Copy link
Owner

JaKooLit commented Mar 7, 2024

Oh my bad... and you were right...

I just reviewed again... thanks alot

however, in the future, kindly make a PR on the development branch.

thanks alot

@cbaconnier cbaconnier deleted the fix-sddm-prompt-loop branch March 7, 2024 20:53
@cbaconnier
Copy link
Contributor Author

No problem, I will do!

Have a great day / evening and thanks for what you do for the community. Much appreciated ! ❤️

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.

None yet

2 participants