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

Add cli support #44

Open
wants to merge 32 commits into
base: main
Choose a base branch
from
Open

Add cli support #44

wants to merge 32 commits into from

Conversation

greygoo
Copy link

@greygoo greygoo commented Oct 17, 2024

This PR adds a cenity function that will take options working with zenity to create terminal output similar to zenity dialogs.

@@ -61,7 +61,7 @@ encrypt_notification() {
# Secureboot being disabled makes the notification a Yes/No with the preference being to exit
if [ "${secureboot_disabled}" == 1 ]; then
log "[encrypt_notification] secureboot warning shown"
zenity --width=600 --question --icon=security-low-symbolic --title="Warning" --ok-label="Cancel Installation" --cancel-label="I Understand, Proceed Anyway" --text="${preamble}\n\nReason: <b>SecureBoot Disabled</b> and ${reason}\n\n${secureboot_warning}" && exit 1
d --width=600 --question --icon=security-low-symbolic --title="Warning" --ok-label="Cancel Installation" --cancel-label="I Understand, Proceed Anyway" --text="${preamble}\n\nReason: <b>SecureBoot Disabled</b> and ${reason}\n\n${secureboot_warning}" && exit 1
Copy link
Owner

Choose a reason for hiding this comment

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

This will not work. The reason it's structured like this is so the zenity's right most (default) button will be 'Cancel Installation' and the left most (normally cancel) button will be 'I Understand, Proceed Anyway.

d will interpret the 'Cancel' as a failure, as abort tik. Whereas the && exit 1 will also exit tik..so this is a functional dead end :)

I guess we need an some kind of flag/variant of d that removes the return code checking so we can preserve this behaviour while working better with cenity (love the name btw)

Maybe d-opt the same way we have prun-opt which does the same for prun as we now need for d

Dont' think this problem is serious enough to block merging, but will need to be addressed before the next tik release.

@@ -10,7 +10,7 @@ fi

if [ ! -z "$(ls -A ${mig_dir})" ]; then
log "existing backup found"
zenity --question --no-wrap --cancel-label="No, Delete Backup" --title="Existing user backup detected" --text="These users can be restored to the new installation\n\nWould you like to use this backup?"
d --question --no-wrap --cancel-label="No, Delete Backup" --title="Existing user backup detected" --text="These users can be restored to the new installation\n\nWould you like to use this backup?"
Copy link
Owner

Choose a reason for hiding this comment

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

Same as the similar comment in 15-encrypt, needs a d-opt or similar function because we're relying on the non-0 return code to inform us that the backup needs to be deleted, not that we want to stop the install (the default behaviour of d)

@@ -105,7 +109,7 @@ if [ -z "${skipbackup}" ]; then
if [ "${legacy_aeon}" == 1 ]; then
d --info --width=300 --height=300 --icon=distributor-logo-Aeon-symbolic --no-wrap --title="Message from the Aeon Team" --text="We'd like to thank you for adopting openSUSE Aeon so early in it's development,\nbefore we fully understood what we were building or how we wanted it to look\n\nWe are sorry that you need to reinstall your system\n\nThank you so much for your support.\nWe hope you enjoy the new look openSUSE Aeon"
fi
zenity --question --no-wrap --title="Backup users from the existing install?" --text="These users will be restored to the new installation."
d --question --no-wrap --title="Backup users from the existing install?" --text="These users will be restored to the new installation."
Copy link
Owner

Choose a reason for hiding this comment

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

Same as the similar comment in 15-encrypt, needs a d-opt or similar function because we're relying on the non-0 return code to inform us that the user doesn't want to backup users not that we want to stop the install (the default behaviour of d)

case $retval in
0)
prun /usr/sbin/cryptsetup luksAddKey --key-file=${tik_keyfile} --batch-mode --force-password "${cryptpart}" <<<"${key}"
return 0
;;
1|255)
zenity --question --text="Do you really want to quit?" && exit 1
d --question --text="Do you really want to quit?" && exit 1
Copy link
Owner

Choose a reason for hiding this comment

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

This ends up being a forced abort of the install because if you click "No", informing the installer you dont want to exit, you end up triggering d's error handling that makes the installer think you want to exit.. :)

We really need that d-opt function mentioned in other comments :)

@greygoo
Copy link
Author

greygoo commented Oct 17, 2024

Indentation is 2 spaces instead of 4. Needs to be fixed


c_key() {
echo -e "\tPress key to continue"
read
Copy link
Owner

Choose a reason for hiding this comment

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

read -n 1 -s -r would be better here, plain read lets you type junk and needs an Enter to continue

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