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

return function #59

Merged
merged 6 commits into from
Feb 24, 2017
Merged

return function #59

merged 6 commits into from
Feb 24, 2017

Conversation

papajoker
Copy link
Collaborator

@papajoker papajoker commented Feb 22, 2017

  • can choise return function menu in parameter 3
  • add fonctions path in logs (special dev)
    if path too long ? can reduce, last 4 for exemple : local _fpath="${FUNCNAME[*]:1:4}()"
02/22/17 18:23:25 mount_current_partition   --mount_current_partition()<-mount_partitions()<-prep_menu()<-select_device()<-prep_menu()<-main_menu_online()<-main()]
02/22/17 18:23:33 Create swap partition: swapon   --make_swap()<-mount_partitions()<-prep_menu()<-select_device()<-prep_menu()<-main_menu_online()<-main()]
02/22/17 18:23:41 [mkfs.vfat -F32 /dev/sda1][1]  --mount_partitions()<-prep_menu()<-select_device()<-prep_menu()<-main_menu_online()<-main()]
02/22/17 18:23:47 mount /dev/sda1 /mnt/boot/efi   --mount_partitions()<-prep_menu()<-select_device()<-prep_menu()<-main_menu_online()<-main()]

# path too long ;)
02/22/17 18:28:15 [rank mirrors][1]  --configure_mirrorlist()<-configure_mirrorlist()<-install_base_menu()<-main_menu_online()<-check_base()<-main_menu_online()<-check_base()<-main_menu_online()<-prep_menu()<-main_menu_online()<-check_base()<-main_menu_online()<-prep_menu()<-prep_menu()<-lvm_create()<-lvm_menu()<-prep_menu()<-luks_menu()<-luks_open()<-luks_menu()<-prep_menu()<-mount_partitions()<-prep_menu()<-select_device()<-prep_menu()<-main_menu_online()<-main()]

@oberon-manjaro
Copy link
Collaborator

why do we need to print the path to log?

@papajoker
Copy link
Collaborator Author

papajoker commented Feb 22, 2017

You have added $FUNCNAME in some calls to the function log
commit
I only automate this functionality

Otherwise it would be possible to validate it only with a parameter manjaro-architect -p ?

@oberon-manjaro
Copy link
Collaborator

Sorry to bother you. I just would like to understand ... learning daily on the way here ...
So ($_main_menu_online) is some magical function I very likely just don't know? 😜

@papajoker
Copy link
Collaborator Author

papajoker commented Feb 22, 2017

oops,
bad copy/paste
call the function "$_function_menu" pass by parameter 3, if empty call main_menu_online

@oberon-manjaro
Copy link
Collaborator

Ok, that's what I tought 😉

@oberon-manjaro
Copy link
Collaborator

When you say "optional arg" do I understand you right that we can introduce the function like this without the need to add a third arg to every present call for check_for_error? So even though both args 2 and 3 we need to specify at least two for the function to be happy, right?

@papajoker
Copy link
Collaborator Author

papajoker commented Feb 22, 2017

yes optional :
in other language we write:

check_for_error(_msg, _err=0, _function_menu=null)

check_for_error "title" == check_for_error "title" 0 main_menu_online
check_for_error "title" 1 == check_for_error "title" 1 main_menu_online
check_for_error "title" $? config_base_menu

in code is :

  local _err="${2:-0}"
  local _function_menu="${3:-main_menu_online}"

if $2 not set $2 = 0 error code
if $3 not set $3= main_menu_online function

so if you want only log you can write check_for_error "info for log $var"

Parameter Substitution Advanced Bash-Scripting Guide

@Chrysostomus
Copy link
Owner

Is this ready to be merged?

@oberon-manjaro
Copy link
Collaborator

oberon-manjaro commented Feb 23, 2017

I'm currently testing it.
I think that the function path is too much information and too messy:

It gets quite unreadable and I think it's not needed anyway.
The occasions where I had inserted $FUNCNAME where only when the name of the function was informative about what had just happened ... ;)

@oberon-manjaro
Copy link
Collaborator

@papajoker optional args are awesome. Thank you!

@oberon-manjaro
Copy link
Collaborator

Also the thing with _canceldlg var doesn't really seems to work out. I just ended up in some kind of a confused menu where I was not able to quit the installer but was sent back to main_menu and behaviour of cancel buttons became somehow confused...

@papajoker
Copy link
Collaborator Author

remove FUNCNAME[*] in log
but I left two functions only if error, because for me, who does not know the code, it helps to locate the problem in code source.

for _canceldlg, Yes, this feature should not exist, normally it should never have check_errors right after a dialog, or then pass it to second parameter 0 and not $?

@oberon-manjaro
Copy link
Collaborator

Makes sense. So we need to find and amend those instances and only use one arg.
And In case we'd still want to use an arg3 in one of these places we should use something like
check_for_error some-text 0 some-function ?

@papajoker
Copy link
Collaborator Author

papajoker commented Feb 23, 2017

yes, pass 0 is the solution
view this bug https://forum.manjaro.org/t/unstable-manjaro-architect-beta-testing/16010/406 , But I am unable to locate this dialog in the code (install efi/esp, cancel for format), that's why I created this variable _canceldlg : i suppose that if errcode=1 and file $ERR empty, its not a error but a cancel dialog.
I do not know if there are other cases

@oberon-manjaro
Copy link
Collaborator

It's here: https://github.com/Chrysostomus/manjaro-architect/blob/master/lib/util-disk.sh#L439
yes, exactly a situation like mentioned.
So we'll need to change that one to
check_for_error "mkfs.vfat -F32 ${PARTITION}" "0"

@papajoker
Copy link
Collaborator Author

papajoker commented Feb 23, 2017

check_for_error wait integer as param two, quote not necessary

the error here is the structure, the AND : && ...

if DIALOG " $_PrepMntPart " --yesno "$_FormUefiBody $PARTITION $_FormUefiBody2" 0 0 ; then
    mkfs.vfat -F32 ${PARTITION} >/dev/null 2>$ERR
    check_for_error "mkfs.vfat -F32 ${PARTITION}" $?
else
    check_for_error "skip format efi ${PARTITION}" 0
fi

oberon-manjaro pushed a commit that referenced this pull request Feb 23, 2017
@oberon-manjaro
Copy link
Collaborator

I think I have adjusted them all now. Should be fine to remove _canceldlg var now.
Next I will check all places where we have instances of 2>$ERR and add check_for_error if still missing.
Then I think this should be ready. :)

@papajoker
Copy link
Collaborator Author

papajoker commented Feb 23, 2017

DIALOG " $_PrepMntPart " --yesno "$_FormUefiBody $PARTITION $_FormUefiBody2" 0 0 && mkfs.vfat -F32 ${PARTITION} >/dev/null 2>$ERR
check_for_error "mkfs.vfat -F32 ${PARTITION}" 0

NO , too speed ;)
view my prev post
here if i have a real error (in $ERR), we pass always 0 to check_for_error


_canceldlg removed now if errcode !=0 always call fonction_menu

@oberon-manjaro
Copy link
Collaborator

ah, grrr, yes of course! ;)

papajoker referenced this pull request Feb 24, 2017
@Chrysostomus Chrysostomus merged commit 523114a into Chrysostomus:master Feb 24, 2017
@papajoker papajoker deleted the fonction branch February 24, 2017 14:31
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.

3 participants