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

WIP: change CLI backend to typer #78

Closed
wants to merge 6 commits into from

Conversation

abhijeetSaroha
Copy link
Collaborator

Created the new CLI file with typer named with makim_cli.py and change the __main__.py accordingly.

As issue completely solved, I will change the name to cli.py.

Solves #76

@abhijeetSaroha
Copy link
Collaborator Author

abhijeetSaroha commented Dec 27, 2023

The one problem that I am currently facing is for --clean as it is used in only two run commands of .makim.yaml file. Could you please tell me more detail about this?

Because in previous cli.py file also there is no mention of --clean flag.

Copy link
Member

@xmnlab xmnlab left a comment

Choose a reason for hiding this comment

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

I will test your branch locally.
But at the mean time I added some notes here

pyproject.toml Outdated Show resolved Hide resolved
@@ -0,0 +1,203 @@
"""
Copy link
Member

Choose a reason for hiding this comment

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

@abhijeetSaroha

Why you are creating a new module, instead of using cli.py?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For now, I have created a new module. When it will complete, I will remove the previous cli.py and change this one to cli.py .
I do this so that functioning of makim continue until new cli with typer fully created.

But if you want I will remove the previous one.

Copy link
Member

Choose a reason for hiding this comment

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

yep .. feel free to remove it ... you always can check the current version on github .. so no worries about that file :)

@xmnlab
Copy link
Member

xmnlab commented Dec 28, 2023

The one problem that I am currently facing is for --clean as it is used in only two run commands of .makim.yaml file. Could you please tell me more detail about this?

Because in previous cli.py file also there is no mention of --clean flag.

Makim is a tool for dynamic creating a CLI from specifications defined in .makim.yaml

--clean is not part of the built-in makim options, instead it is something defined for our .makim.yaml .. that config file is used for makim development only ... it is not something that would be used out side our development. Any other projects that use makim, can define their own set of group and targets.

src/makim/makim_cli.py Outdated Show resolved Hide resolved
echo(help_text)


def format_help_text(description, usage, epilog, target_help):
Copy link
Member

Choose a reason for hiding this comment

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

you probably don't need to reimplement the help ..
you should use the one from typer .. in another words, you don't need to create it manually ..
just need to create the functions with the correct arguments for each target, and add the correct help text for that, and typer will create the help menu for you. similar to what I shared in the issue.

src/makim/makim_cli.py Outdated Show resolved Hide resolved
@xmnlab
Copy link
Member

xmnlab commented Dec 28, 2023

thank you @abhijeetSaroha for working on that!
I just added more comments here.

@felipepaes
Copy link

Hi @abhijeetSaroha, pleased to meet you!

I will be helping you with makim PRs since @xmnlab won't be able to test things on his computer for the next few days.

Please let me know if you need anything, whether it's for testing the PRs, exchanging/sharing ideas, or discussing opinions about the project.

I'm also on the Discord server. My timezone is GMT-3, but feel free to contact me at any time, ok?

@felipepaes
Copy link

I'm working on a feature (issue 30 ) for makim as well, so I will need to learn more about Typer. Currently I have added new options to the parser so that we could have something like this:

makim cron run backup
makim cron install backup
makim cron remove backup

Basically I have added a subcommand called cron which can add makim commands as cron jobs that are executed by the system via cron at specific set times.

Since we are going for Typer, I will be reading this piece of documentation about subcommands with Typer

Please let me know if you have any resources about Typer that you would recommend.

@xmnlab
Copy link
Member

xmnlab commented Dec 29, 2023

That looks nice, probably something that I would need to do for envers as well

@abhijeetSaroha
Copy link
Collaborator Author

Hi @abhijeetSaroha, pleased to meet you!

I will be helping you with makim PRs since @xmnlab won't be able to test things on his computer for the next few days.

Please let me know if you need anything, whether it's for testing the PRs, exchanging/sharing ideas, or discussing opinions about the project.

I'm also on the Discord server. My timezone is GMT-3, but feel free to contact me at any time, ok?

Pleased to meet you too @felipepaes sir.

@abhijeetSaroha
Copy link
Collaborator Author

@felipepaes I am not able to dynamically create command and create help text automatically.
As @xmnlab , give example here.

Can you give some more idea about this?

@felipepaes
Copy link

@felipepaes I am not able to dynamically create command and create help text automatically. As @xmnlab , give example here.

Can you give some more idea about this?

Sorry @abhijeetSaroha I was at a retreat with family.

So, on that example he's getting the help from the targets defined in .makim.yaml, as an example he's already defining it as dictionary named targets.

Did you mean to generate the help text based on the .makim.yaml targets' args or the command help text?

@felipepaes
Copy link

Typer should automatically fetch the arguments and options when you use the --help option, but you can also pass a general cli help text when instantiating the Typer object.

app = typer.Typer(help="Awesome CLI user manager.")

source

@xmnlab
Copy link
Member

xmnlab commented Jan 3, 2024

@abhijeetSaroha could you share more about the error you have? is there your code already here on github?
if so I can test it locally

@app.callback(invoke_without_command=True)
def main(
# Common Options
help: bool = Option(
Copy link
Member

Choose a reason for hiding this comment

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

you probably don't need to specify help manually.


args = {
'dry_run': dry_run,
'help': help,
Copy link
Member

Choose a reason for hiding this comment

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

you also probably don't need to use help here
let's have typer to handle the help automatically

'version': version,
}

makim.load(makim_file)
Copy link
Member

Choose a reason for hiding this comment

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

not sure if that is the write way to work with makim and typer together

in this main function you probably should allow makim-file, and version.

for the other commands you will need to implement (for each one) verbose, dry-run and makim-file

the target you can set manually for each one, because each command will trigger just one function

does it make sense?

Copy link
Member

Choose a reason for hiding this comment

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

btw, let me know if this one is too complex for you ... and I can work on this one.
this one is really tricky because it is not using the default workflow for typer

Copy link
Collaborator Author

@abhijeetSaroha abhijeetSaroha Jan 3, 2024

Choose a reason for hiding this comment

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

Let me try one more time, then I will report to you.

@xmnlab
Copy link
Member

xmnlab commented Jan 12, 2024

superseded by #82

@xmnlab xmnlab closed this Jan 12, 2024
@abhijeetSaroha abhijeetSaroha deleted the cli-typer branch January 15, 2024 16:02
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