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(cli): support trivial cli args like --version #47

Merged
merged 5 commits into from
Mar 19, 2024

Conversation

dbarnett
Copy link
Contributor

@dbarnett dbarnett commented Mar 1, 2024

Fixes #42.

The advantages are that if you pass standard arguments like --version it will interpret those normally, and if you pass totally invalid args like --pod-bay-doors=open it will tell you they're unrecognized instead of seeming to accept them and launching normally.

The secret was to ignore Tauri's brittle cli config integration and just use clap directly. I asked how to get Tauri's hooks to behave normally (https://discord.com/channels/616186924390023171/1212281820302016552) but didn't get an answer, and anyway don't see any advantage to trying to use Tauri's helpers vs. using clap directly.

Copy link

changeset-bot bot commented Mar 1, 2024

🦋 Changeset detected

Latest commit: 63e6134

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@tablex/core Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Mar 1, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
tablex ⬜️ Ignored (Inspect) Visit Preview Mar 19, 2024 0:31am

@dbarnett
Copy link
Contributor Author

dbarnett commented Mar 1, 2024

Hmm, commitlint seems broken:

node_modules/@commitlint/cli/lib/cli.js:129
        throw err;
        ^

TypeError: format is not a function

Not sure if this is a cryptic error somehow indirectly triggered by something in my PR or the version it's using is just completely broken irrespective of my PR.

Also FYI, I noticed that the automation isn't deleting the --empty changeset files and those are just kinda piling up: https://github.com/kareemmahlees/tablex/tree/master/.changeset.

@kareemmahlees
Copy link
Owner

Hmm, commitlint seems broken:

I managed to reproduce it locally using act, and pushed a fix

Will try to fix the problems with the other workflows.

@kareemmahlees
Copy link
Owner

Fixed Vercel's deployment issue

@kareemmahlees
Copy link
Owner

Also FYI, I noticed that the automation isn't deleting the --empty changeset files and those are just kinda piling up: https://github.com/kareemmahlees/tablex/tree/master/.changeset.

Huh, that's strange, it worked as expected in previous PRs.
Maybe because I didn't wait for the workflows to finish!?

I will delete them manually and then monitor the behavior closely.

@kareemmahlees
Copy link
Owner

How should the end user use TableX as a CLI?

@dbarnett
Copy link
Contributor Author

dbarnett commented Mar 1, 2024

By "use it as a cli" I just mean that like any installed program it can be invoked from a terminal, not only from your OS's launcher, and for anything that can be invoked as a command it's useful if it at least tells you you're invoking it with nonsense args.

As far as what actual useful options it could support, #46 was one thing I'd imagined from the start (parallel to the way you can invoke vscode on a particular path from your terminal as code someproject/ as an alternative to launching a blank vscode session and navigating through the UI to find and open a particular project dir).

--help could also point to the website / basic docs and any other explanations of whatever ways to configure and use TableX, just so it's all self-contained that if you have table-x installed on your system path but forgot what it does you can ask it.

@kareemmahlees
Copy link
Owner

I didn't mean what are the use cases, I meant how the user will invoke the table-x command.
What should be put in the $PATH env var so that the user can execute table-x --help?

@dbarnett
Copy link
Contributor Author

dbarnett commented Mar 1, 2024

Oh, for me on Linux, installing the downloaded .deb added a table-x executable on my path (/usr/bin/). Is your point that on other platforms the installation/usage steps are less self-explanatory about how to launch it from a terminal?

@kareemmahlees
Copy link
Owner

Yes, because for example on Windows nothing is added to the path when I install it.
I think maybe that is what Tauri is doing under the hood!

@dbarnett
Copy link
Contributor Author

dbarnett commented Mar 1, 2024

Right, so I think what you're saying is that the advantages would be more geared towards some platforms than others, which doesn't seem like a problem but maybe you do have specific concerns around that?

@kareemmahlees
Copy link
Owner

Yeah my general concerns is that it would be definitely better for the feature to be available for all platforms.
And I think if Tauri offers that, then it might be worth trying a little bit harder with Tauri's integration with Clap for the sake of multie platform support, wdyt ?

@dbarnett
Copy link
Contributor Author

dbarnett commented Mar 2, 2024

I don't think Tauri's integration has anything to do with whether you can invoke programs from cli. It's just some thin convenience wrappers for handling whatever args you pass, with some kinda strange behavior defaults. You're thinking somehow the Tauri config helpers have something to do with making cli convenient to use on Windows?

@kareemmahlees
Copy link
Owner

There are a couple of strange behaviors I am noticing here:

  • When in debug build:
    • Using plain clap:
      • Working as expected
    • Using Tauri helpers:
      • Working as expected ( Although it is less verbose and will require further parsing of the matches)
  • When in release build:
    • Using Tauri helpers:
      • Not outputting anything
    • Using plain clap:
      • Not outputting anything

I think the issue with release builds on Windows is related to this tauri-apps/tauri#4118, so Windows users like me will not be able see any output on the terminal but will be able to use the CLI.
Also, I don't know if it is a Windows-specific thing or not but the installed build is not added to the PATH automatically.

I think maybe we can include this in the documentation with some notes for Windows users about the CLI behaviour and how they should add the app to their `PATH, wdyt?

@dbarnett
Copy link
Contributor Author

Sure, we can add some documentation notes if you're good with the changes in general.

Are you thinking the issue you linked would explain "When in release build: Using plain clap: Not outputting anything", even though the bug is against Tauri and the repro is plain clap? That does sound worth digging into before merging anything.

@kareemmahlees
Copy link
Owner

It sounds pretty sane actually, because if Tauri detaches from the console/terminal when running the GUI then even the simplest println will not work needless to say clap output.

But I found a workaround here tauri-apps/tauri#8305 (comment), and it works like a charm!

@kareemmahlees
Copy link
Owner

Could you confirm that everything is still working as expected on your OS, and if you have any other comments lmk.

@dbarnett
Copy link
Contributor Author

Actually as-is it gives me errors "use of undeclared crate or module `windows`" (where the two helper functions are still unconditionally defined and reference windows). Adding #[cfg(windows)] guards on the function definitions too fixes that for me.

@kareemmahlees kareemmahlees merged commit ea9bfcd into kareemmahlees:master Mar 19, 2024
10 checks passed
@kareemmahlees kareemmahlees mentioned this pull request Mar 19, 2024
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.

TableX ignores cli arguments
2 participants