search

Julien-cpsn / ATAC

A simple API client (postman like) in your terminal
https://atac.julien-cpsn.com/
MIT License
1.86k stars 81 forks source link

feat: (draft) import from cURL #49

Closed jwtly10 closed 4 months ago

jwtly10 commented 5 months ago

This is a WIP implementation of importing curl files into ATAC - requested in https://github.com/Julien-cpsn/ATAC/issues/48

It should support all browsers Copy > Copy as cURL dev function, and parse Bearer Tokens, headers, and URL params.

There are some points that should be discussed:

  1. How should we handle a collection for imported curl requests? For now I have just allowed an 'imported' collection to be created or appended to.
  2. For now the request name will be the name of the file that is being imported.
  3. At the moment, only one curl req per file is supported, but I believe this can be extended quite easily.
  4. I have supported curl import for ANY file type other than .json (which is for postman collection imports). Should we only support .curl for example?

There may be some cases I have missed as this is a very draft impl while the above are addressed.

Here is a demo:

https://github.com/Julien-cpsn/ATAC/assets/39057715/b287762b-be2a-4b07-980b-ddee112d1082

Thanks!

NachoNievaG commented 5 months ago

This looks great! I would totally use it. As a user, a selector to define in "which collection do you want to import it" prompting the existing collections would be a great UX.

jwtly10 commented 5 months ago

@NachoNievaG Yeah i agree, that sounds like a good idea.

I have updated the pull request to allow picking where you import the request. Here is how it looks so far:

https://github.com/Julien-cpsn/ATAC/assets/39057715/2c2a11ec-e451-4897-8f1e-e0796e77ba32

Julien-cpsn commented 4 months ago

@jwtly10 @NachoNievaG Hello guys, sorry I have been really busy lately. I'll try to review both of your PRs this week.

Thanks for your personnal investment in this project

Julien-cpsn commented 4 months ago

Hello @jwtly10, great work here!

Concerning the cURL import part, everything seems ok (did not have time to locally test it yet) However, regarding the "Where to save the request" popup, I personnally do not think it is a good way of doing things. It involves too many UI/UX elements for something that doesn't need to. Regarding TUI apps, my mind is set on "less UI is best".

However², the idea behind this popup is great and I think we should implement it (but not this way). I suggest a simplier thing that will be more practicle for everyone: a CLI argument.

(Also, I'd like the user to precise which format he's importing from, so this way will be better)

At the end we get something like this:

atac import curl my_collection/my_request ./import_tests/example_curl

What do you think about it? (also @NachoNievaG)

jwtly10 commented 4 months ago

@Julien-cpsn I agree completely - reading just from cmdline will be a much leaner implementation too.

It was difficult to make any decisions without your feedback so thanks for getting back.

I really like the project and I use it daily so I have no problem contributing back where I can 👍🏽

I'll fix the pr over the next day or so.

Julien-cpsn commented 4 months ago

@Julien-cpsn I agree completely - reading just from cmdline will be a much leaner implementation too.

It was difficult to make any decisions without your feedback so thanks for getting back.

I totally understand no worries. I hope you are enjoying the new v0.15.0 features!!

Big updates are coming :)

I really like the project and I use it daily so I have no problem contributing back where I can

Thanks man, I put my soul into it 😂 Do not hesitate to provide feedback on commits, pr, discussions

I'll fix the pr over the next day or so.

I can do it if you like

NachoNievaG commented 4 months ago

Although I agree as well, a few UI/UX enhancements seems fair to be able to handle all CRUD available. There are a lot of TUI apps' users that are TUI users to avoid CLI implementations. That being said, I do like a lot more CLI commands, given that would also be able to handle a lot of new scenarios. Using your example @Julien-cpsn , this comes instantly to my mind:

# this would go to each existing file to export
atac import curl my_collection ./import_tests/...
# or more bin flag oriented, in which "import_tests" is a dir.
# as -r for being recursive.
# -k --kind or -t --type to define the import kind/type.
atac import -r --kind curl my_collection ./import_tests

PS: I refer curl as a kind given that it's type would be "shell/sh"

jwtly10 commented 4 months ago

@NachoNievaG I think that TUIs are nice, but after thinking about it I do agree in this case its a little bit over kill. While this is not my code base so perhaps my understanding of the best way to implement this was wrong - but my solution felt quite hacky, especially for something that only has a single use case.

I do also think command line argument can be extremely useful for scripting too (i dont have this use case but CLA maintains the possibility).

@Julien-cpsn I have made the change so I believe this PR is ready for a final review. It doesn't support a recursive flag as @NachoNievaG mentioned but this should be relatively simple to implement if its something you see value in.

Julien-cpsn commented 4 months ago

Hello @jwtly10 @NachoNievaG

I finalized the PR here https://github.com/Julien-cpsn/ATAC/pull/66 (I tried to commit to @jwtly10 's repo, but did not succeed)

I also implemented the --recursive option along with --max-depth. It also supports all content-types except forms (but with more time it's possible to do it)

Here's the video:

https://github.com/Julien-cpsn/ATAC/assets/64107022/926a4af5-8e00-410a-bdf1-c52e753f9e0d

Any feedback?

Julien-cpsn commented 4 months ago

(the merge was a fail)