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

api: add remove_deprecated_app and userinput_func functions #2419

Merged
merged 2 commits into from
Aug 9, 2023

Conversation

theofficialgman
Copy link
Collaborator

@theofficialgman theofficialgman commented Aug 4, 2023

also remove deprecated apps:
Email Checker
Pi-Apps Terminal Plugin (python)
Sysmon
BlockPi

work towards #2401

I have tested this

case $(ps -o stat= -p $$) in
  *+*) # Running in foreground

within a pi-apps runonce and it does work to detect when it is running as part of a foreground process.
We need to be running in a foregroud process with GUI available in order to prompt the user to chose to uninstall the application before it is removed.

the process is foreground with GUI available when the runonce-entries are run as part of the manage-daemon updater as well as after any install/uninstall from the pi-apps GUI.

@theofficialgman theofficialgman marked this pull request as draft August 4, 2023 23:27
@theofficialgman
Copy link
Collaborator Author

this is still a draft as I am looking for feedback/cleanup for userinput_func

@theofficialgman theofficialgman force-pushed the deprecated-app-update branch 5 times, most recently from 8bdffe0 to 3babb99 Compare August 5, 2023 13:28
also remove deprecated apps:
Email Checker
Pi-Apps Terminal Plugin (python)
Sysmon
BlockPi
@theofficialgman theofficialgman marked this pull request as ready for review August 7, 2023 18:50
@Botspot
Copy link
Owner

Botspot commented Aug 9, 2023

Reviewed now. Sorry for the delay - I have a lot of stuff to do in the real world.

userinput_func looks rather flexible. Do you have any other specific places in mind to replace current dialogs?
My main two concerns are:

  • Feeling constrained to use the few options userinput_func offers, resulting in potentially suboptimal dialog designs for unforeseen situations. (Take the Arduino color choice dialog for an example of how yad freedom resulted in a better design)
  • Muddying the water if we try to get app-contributors to use this. Now they have to understand when to use yad and when to use userinput_func. For people in the future who try to understand the source code, there would now be two general dialog commands to watch out for and understand. (Cue XKCD 927)

To be clear, neither of those concerns make me disapprove userinput_func, but hopefully they help show why I'm not as enthusiastic about it as you are.

@theofficialgman
Copy link
Collaborator Author

userinput_func looks rather flexible. Do you have any other specific places in mind to replace current dialogs?

It isn't intended to replace current dialogs.

  • Feeling constrained to use the few options userinput_func offers, resulting in potentially suboptimal dialog designs for unforeseen situations. (Take the Arduino color choice dialog for an example of how yad freedom resulted in a better design)
  • Muddying the water if we try to get app-contributors to use this. Now they have to understand when to use yad and when to use userinput_func. For people in the future who try to understand the source code, there would now be two general dialog commands to watch out for and understand. (Cue XKCD 927)

To be clear, neither of those concerns make me disapprove userinput_func, but hopefully they help show why I'm not as enthusiastic about it as you are.

See above, it's not a replacement for custom dialogs. It's a tool for script writers to use at their choice not needing to understand yad and duplicate work. An earlier version userinput_func is already shipped in multiple scripts and I have seen multiple PRs from contributors also copy my function (verbatim) and put it into their own scripts. It just makes sense to clean it up and unify it so that

@Botspot Botspot merged commit 8138c9c into master Aug 9, 2023
3 checks passed
@Botspot Botspot deleted the deprecated-app-update branch August 9, 2023 17:11
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