linux-cli-community icon indicating copy to clipboard operation
linux-cli-community copied to clipboard

Going back to the menu

Open Y0lan opened this issue 4 years ago • 8 comments

added a "go back to menu entry" to all the entry where it made sense to add it. Now you can configure all the app without relaunching it.

I also cleaned up some things that I can easily revert back if needed.

  1. New Exit entry into main menu
  2. Type menu into the proton username to go back to menu
  3. New option to go back to the menu added to each list
  4. (optional but thought it would make sense) since Plus and Visionary have the same functionality, I grouped them together.

Y0lan avatar Apr 14 '20 08:04 Y0lan

(optional but thought it would make sense) since Plus and Visionary have the same functionality, I grouped them together.

Please remove this. This is done intentionally to avoid confusion of users ("I'm on visionary, why doesn't it support this plan?"). Not everyone knows that they are the same. As you've seen, it sets the same value anyway, it's just to not cause confusion.

Rafficer avatar Apr 14 '20 08:04 Rafficer

Sorry, I should not have changed this. :)

Y0lan avatar Apr 14 '20 08:04 Y0lan

These changes mess up protonvpn init:

image

It's also not possible anymore to choose visionary as plan, it just throws you back to the menu. Same if you choose it during initialization:

image

Rafficer avatar Apr 15 '20 22:04 Rafficer

I did not take into account the init option, good catch ! :sweat_smile: I've tested it now, and it should work.

I added an 'Init' boolean argument to the 3 functions run during initialisation to display or not, and enable / disable the menu option.

I should have thought of this before my pull request. I hope it's ok now ?

Y0lan avatar Apr 16 '20 09:04 Y0lan

Instead of adding a new boolean, wouldn't it make more sense to utilize write=True, which already exists?

Rafficer avatar Apr 16 '20 10:04 Rafficer

Instead of adding a new boolean, wouldn't it make more sense to utilize write=True, which already exists?

Good question. I don't think so since the relation between write and init mode isn't direct. It could be a hack tough, but adding a boolean and gaining in readability is a good compromise I think. But it's your software, so if you want me to remove it no problem.

Y0lan avatar Apr 18 '20 10:04 Y0lan

I'm actually thinking about whether it would be better to rename it to init, as that's actually what matters.

It just doesn't make sense to have 2 booleans that are always the same in the 2 different cases.

Let me submit a PR to your branch later. :)

Rafficer avatar Apr 18 '20 10:04 Rafficer

I'll test it tomorrow !

Y0lan avatar Apr 18 '20 22:04 Y0lan