The application closes after each choice

amtoine commented 1 year ago

Hello there :wave: :yum:

networkmanager_dmenu is super handy to connect to the internet :star:

however, i think there is one thing i find quite hard to work with in a "Desktop Environment experience"

What is happening now

dmenu and the application itself closes completely after each choice the user makes !

example

when i rescan the network, the application closes and does not allow me to choose a network among the new scanned ones

What i expected to happen

not close the application after each choice of the user, maybe add an "exit" option and repon dmenu

example

when i rescan the network, a notification would appear as normal when done, but dmenu would reopen with the newly scanned network entries, letting me choose among all of them without going through rofi or dmenu_run to open networkmanager_dmenu again

i think that would be really cool to have networkmanager_dmenu not be a "one-shot" dmenu command :yum: would you accept a PR on such a feature? :open_mouth:

cheers :wave: :tada:

firecat53 commented 1 year ago

PR would be great! Sounds reasonable, although keep in mind that the rescan time can be long, so it might be a surprise to have it open again. Would a notification even be necessary in that case? I also have not found a good way to ensure that the rescan is actually completed. I'll have to reopen the app a few times before it gets populated. If I remember right, the notification triggers when the scan is complete but that doesn't necessarily mean it will have the new values yet. It's been awhile though, to be honest.

Thanks for the kind words!

amtoine commented 1 year ago

got it regarding the notifications and the rescan :+1:

i might try to have a look and will let you know through a PR :wink:

amtoine commented 1 year ago

i had a look at your develop branch and it looks really great :+1:

i think i did not understand the problem at the time i wrote my PR :thinking: the thing really is that we do not know the time it takes for a rescan to complete... then my changes are not that great at all, i agree :confused: i assumed the rescan was instant and reopened dmenu immediately, which is not correct

thoughts about `develop#ee9852a`

the exit option

i do not know if that part of the changes i proposed is usefuil i thought it was cool, but it might be overkill and only usefull on a rescan :thinking: i think only time will give us an insight on that :wink:

the rescan timeout

the 5 seconds time.sleep look enough for a wifi rescan however, it might be a bit too strong to say "wifi scan complete" right? because we do not have this information when the 5 seconds counter is over...

i can see 2 main ways to slightly improve that part:

change the message to something less strong, e.g. "Wifi scan running for {delay} seconds..." and "Wifi scan should be complete". We could even throw a progress bar to indicate the time remaining if you'd like to go fancy :wink:
- :+1: it is less strong and more accurate
- :-1: it is more verbose
could it be possible to know when the rescan is complete? then the "wifi scan is running..." could stay active as long as the wifi scan has not been completed and "Wifi scan complete" could be thrown only when meaningfull :thinking:
- :+1: it looks the best option to me, 'cause it would be super accurate with what happens in the background
- :-1: it might be impossible

moreover, it could be great, regardless of the plan, to let the notification up on the screen for the whole scan time, for instance with the following change

diff --git a/networkmanager_dmenu b/networkmanager_dmenu
index 8393279..510dd43 100755
--- a/networkmanager_dmenu
+++ b/networkmanager_dmenu
@@ -194,7 +194,7 @@ def rescan_wifi():
             try:
                 dev.request_scan_async(None, rescan_cb, None)
                 LOOP.run()
-                sleep(5)
+                sleep(RESCAN_DELAY)
                 notify("Wifi scan complete")
                 main()
             except gi.repository.GLib.Error as err:
@@ -841,7 +841,7 @@ def notify(message, details=None, urgency="low"):
     """Use notify-send if available for notifications

     """
-    args = ["-u", urgency, "-a", "networkmanager-dmenu", message]
+    args = ["-u", urgency, "-a", "networkmanager-dmenu", message, "-t", str(RESCAN_DELAY * 1000)]
     if details is not None:
         args.append(details)
     if is_installed("notify-send"):
@@ -898,10 +898,11 @@ def main():
     """Main. Enables script to be re-run after a wifi rescan

     """
-    global CLIENT, CONNS, LOOP  # noqa pylint: disable=global-variable-undefined
+    global CLIENT, CONNS, LOOP, RESCAN_DELAY  # noqa pylint: disable=global-variable-undefined
     CLIENT = NM.Client.new(None)
     LOOP = GLib.MainLoop()
     CONNS = CLIENT.get_connections()
+    RESCAN_DELAY = 5

     run()

i hope that will be usefull to you in some way :relieved: :yum: cheers :wink: :wave:

firecat53 commented 1 year ago

Thanks for the thoughts! I set a default delay of 5 seconds but added the option to change that in the config file (see README). There is no way that I'm aware of to determine exactly when the data is available from the rescan. The callback function for the rescan is called immediately after the rescan in triggered, so that's not really helpful. Therefore, I'm going to leave the notification messages as is. IMO it's just simpler that way even if it's not 'technically' correct. Please check and let me know if works for you!

amtoine commented 1 year ago

yeah that is cool :+1:

and i also discover the config file, which is super great to be honest :open_mouth:

amtoine commented 1 year ago

maybe add

[nmdm]
# rescan_delay = <time in seconds to adjust the delay in re-opening nmdm following rescan>

to the config.ini.example file :wink:

amtoine commented 1 year ago

thanks a lot :star_struck:

it works just as expected with the AUR package :ok_hand: :muscle:

firecat53 / networkmanager-dmenu