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

feat(just): combine several just commands into toggles or choices #181

Merged
merged 25 commits into from
Jan 12, 2024

Conversation

HikariKnight
Copy link
Member

@HikariKnight HikariKnight commented Jan 1, 2024

This patch, when complete will be the second step in addressing #148 now that we have ugum on all images which solved the need of a standardized way to select between options.

This is something that i will be chipping away at when i am able to.

Naming scheme for just recipes (draft) will be:

  • setup- Used when the recipe provides a way to install and remove something (configuration can also be added here)
  • configure- Used to configure something that is pre-installed on the ublue system (maybe shorten to config-?)
  • install- Used by recipes where there is no functionality other than install
  • remove- used by remove-brew due to the issues we got reported where it broke flatpak, please ignore
  • toggle- switch something on/off like for example toggle-gnome-vrr (if it makes sense maybe ditch toggle- and use configure- instead, unless we keep toggle for things that are specifically on/off switches?)
  • fix- Apply a patch/workaround to something
  • thing A single task that works as a shortcut and needs to be memorable, like update or changelog

Also i have added aliases to provide a shorter name for recipes too where i felt it was warranted.

Input about this proposed scheme is appreciated before merging

Snippet for choice dialogs (adjust for your needs)

#!/bin/bash
bold=$(tput bold)
normal=$(tput sgr0)
echo "${bold}Title${normal}"
echo 'text?'
OPTION=$(ugum choose option1 "option 2")
if [ "$OPTION" == "option1" ]; then
  echo 'stuff'
elif [ "$OPTION" == "option 2" ]; then
  echo 'other stuff'
fi

If you have a lot of options (more than 3?) you can do what i did for nix for readablilty

OPTION=$(ugum choose \
    option1 \
    "option 2" \
    "another option" \
    option4 \
  )

Using ugum (universal gum) will let it generate choice dialogs even when gum is not installed, they will look something like this:

With gum installed, gum will be prioritized:
image

Without gum but with fzf is installed, fallback to fzf:
image

Without both fzf and gum, fallback to pure bash:
image

@HikariKnight HikariKnight self-assigned this Jan 1, 2024
@HikariKnight
Copy link
Member Author

I have left install-nixgl as it is, since it has no other recipes like a removal recipe while with the setup verb one would expect install, remove and configuring something

@HikariKnight
Copy link
Member Author

Distrobox file will require some more thought on what would be the best approach, will do that one later as it is fine as it is for now.

@HikariKnight HikariKnight marked this pull request as ready for review January 3, 2024 11:45
@castrojo
Copy link
Member

castrojo commented Jan 3, 2024

+1 to the changes, let's see what others think. I'd like to shove your reasoning for each verb in a readme at the root of the dir so we have it going forward, but we can do that in a follow up PR. We can also do the distrobox in a follow up PR so that this doesn't keep growing, better to land it in smaller chunks.

Thanks for working on this!

@HikariKnight
Copy link
Member Author

let's see what others think. I'd like to shove your reasoning for each verb in a readme at the root of the dir so we have it going forward, but we can do that in a follow up PR.

It is why i want to see what others think first before it gets fully committed to going forward

Thanks for working on this!

Np! someone has to do it 😄

Copy link
Member

@EyeCantCU EyeCantCU left a comment

Choose a reason for hiding this comment

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

The changes here look great! Definitely a significant improvement over what we have. Just a note - in the future, we should implement quick toggles/selections for these so that they can be used in other places like yafti. I.E., for configure-gamepads, we could have configure-gamepads stock or configure-gamepads ublue

@HikariKnight
Copy link
Member Author

The changes here look great! Definitely a significant improvement over what we have. Just a note - in the future, we should implement quick toggles/selections for these so that they can be used in other places like yafti. I.E., for configure-gamepads, we could have configure-gamepads stock or configure-gamepads ublue

need to play around with it and figure out how it works first in just and do some tests as i was thinking we could do something as simple as just feeding the ugum choice return value to the command or something.

@HikariKnight HikariKnight marked this pull request as draft January 7, 2024 14:46
@HikariKnight HikariKnight marked this pull request as ready for review January 7, 2024 15:44
@HikariKnight HikariKnight force-pushed the HikariKnight-just-reorg branch from f2ba176 to 430c840 Compare January 7, 2024 16:09
Copy link
Member

@EyeCantCU EyeCantCU left a comment

Choose a reason for hiding this comment

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

Everything looks great. Bazzite just needs a couple things adjusted in yafti and then this will be good to go :)

@HikariKnight HikariKnight force-pushed the HikariKnight-just-reorg branch from 48e2346 to c6cb6e6 Compare January 7, 2024 21:34
@noahdotpy
Copy link

noahdotpy commented Jan 9, 2024

I have a proposal:

Can we change the extremely long list of distrobox-[image name here] into one recipe called something like install-distrobox?.
This recipe would have an optional arg to choose the image you want. If this arg is not entered then it would use ugum to let the user choose an option.

This would fit with the naming scheme you have created and would declutter the list of recipes when doing ujust -l

@noelmiller
Copy link
Member

I agree with the above statement! I just added several more distroboxes in #185

We are also talking about wanting to support toolbox as well so users have a choice of which they prefer to use!

@HikariKnight
Copy link
Member Author

I have a proposal:

Can we change the extremely long list of distrobox-[image name here] into one recipe called something like install-distrobox?. This recipe would have an optional arg to choose the image you want. If this arg is not entered then it would use ugum to let the user choose an option.

This would fit with the naming scheme you have created and would declutter the list of recipes when doing ujust -l

will be in separate PR as i have some ideas on how to improve it as i really want to shrink that list too, but i also want to enhance it by letting users apply a name and platform (docker/podman or toolbox) for the distroboxes.
But i will have to draft the change and check what would make sense. 😄

@HikariKnight HikariKnight mentioned this pull request Jan 10, 2024
@HikariKnight HikariKnight force-pushed the HikariKnight-just-reorg branch from e53f9e3 to 195893f Compare January 11, 2024 04:05
@castrojo castrojo enabled auto-merge January 12, 2024 15:24
@castrojo castrojo added this pull request to the merge queue Jan 12, 2024
Merged via the queue into main with commit 1399871 Jan 12, 2024
2 checks passed
@castrojo castrojo deleted the HikariKnight-just-reorg branch January 12, 2024 15:26
@bsherman
Copy link
Contributor

I was just coming here to do a final read and approve! But it's in already!

Thanks for the effort @HikariKnight

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.

7 participants