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

Add Prompt Variables #616

Merged
merged 15 commits into from
Apr 19, 2022
Merged

Add Prompt Variables #616

merged 15 commits into from
Apr 19, 2022

Conversation

theerapatcha
Copy link
Contributor

@theerapatcha theerapatcha commented Jun 18, 2020

Prompt variable is a variable that will prompt for user input every time user sending a request.

Some variables are meant to be changed all the time such as Session ID or OTP code.
Therefore, I think it is better to let the user input the designated value of that variable instead of changing the source code every time.

@Huachao Huachao linked an issue Jun 19, 2020 that may be closed by this pull request
@theerapatcha
Copy link
Contributor Author

@Huachao any chance of this PR getting merged? or is there anything else need to be modified?

Thanks

@Huachao
Copy link
Owner

Huachao commented Jun 24, 2020

@theerapatcha sorry for the late reply, thanks for your contribution, before diving into the implementation, I want to have some discussions with you.

  1. The scope of Prompt Variables (Request scope or file scope)?
  2. The variable definition syntax (Seems related to question 1)
  3. The variable reference syntax (I found you add a preceding $ character before the variable name, actually in my design, this is reserved for system variables only)

@theerapatcha
Copy link
Contributor Author

theerapatcha commented Jun 24, 2020

@theerapatcha sorry for the late reply, thanks for your contribution, before diving into the implementation, I want to have some discussions with you.

  1. The scope of Prompt Variables (Request scope or file scope)?
  2. The variable definition syntax (Seems related to question 1)
  3. The variable reference syntax (I found you add a preceding $ character before the variable name, actually in my design, this is reserved for system variables only)
  1. It is a request scope, our I would say per-request scope just like @note comment tag
  2. By adding // @prompt {varName} or # @prompt {varName},{varName2} just before the request, then, when a user click on Send Request, VS Code will prompt for that varName values.
  3. For now, the prompted input value will replace any reference syntax that matches $prompt.{varName}.

To make it align with your design, I can change it to use just plainly {varName} syntax? Note that, this prompted variable will be override any variable defined precedingly (just for the prompted request).

Thanks for your reply.

@Huachao
Copy link
Owner

Huachao commented Jun 24, 2020

  1. It is a request scope, our I would say per-request scope just like @note comment tag
  2. By adding // @prompt {varName} or # @prompt {varName},{varName2} just before the request, then, when a user click on Send Request, VS Code will prompt for that varName values.
  3. For now, the prompted input value will replace any reference syntax that matches $prompt.{varName}.

For the first question, does the user requires a file-level prompt variable like the credential information for all APIs?

For the second question, I prefer the syntax that one prompt variable one-line, since I want to add an optional placeholder for a variable description with syntax # @prompt variableName [variableDescription] like following:

# @prompt secret My demo service secret

For the third question, I think the {{...}} seems more aligned with other custom variables

@theerapatcha
Copy link
Contributor Author

  1. It is a request scope, our I would say per-request scope just like @note comment tag
  2. By adding // @prompt {varName} or # @prompt {varName},{varName2} just before the request, then, when a user click on Send Request, VS Code will prompt for that varName values.
  3. For now, the prompted input value will replace any reference syntax that matches $prompt.{varName}.

For the first question, does the user requires a file-level prompt variable like the credential information for all APIs?

For the second question, I prefer the syntax that one prompt variable one-line, since I want to add an optional placeholder for a variable description with syntax # @prompt variableName [variableDescription] like following:

# @prompt secret My demo service secret

For the third question, I think the {{...}} seems more aligned with other custom variables

  1. For my use case, i just need a prompt for variable just for a specific request like an OTP from my authenticator app. If there is any need for a file-variable prompt scope, it might be in another PR.
  2. That sounds good, will make that change.
  3. Got it.

@Huachao
Copy link
Owner

Huachao commented Jun 24, 2020

  • For my use case, i just need a prompt for variable just for a specific request like an OTP from my authenticator app. If there is any need for a file-variable prompt scope, it might be in another PR.
  • That sounds good, will make that change.
  • Got it.

The design LGTM, wait for your PR. Thanks for your contribution.

@theerapatcha
Copy link
Contributor Author

I made changes per discussed. You might need to re-review because most of the source code has been changed due to the prompt variable replacement logic which needs to be handled in the Selector class.

Btw, I have no idea why LGTM analysis keeps showing failed and I can't find any log that is related.

@theerapatcha
Copy link
Contributor Author

@Huachao resolved the conflicts! does anything need to be changed? Thanks

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/common/constants.ts Outdated Show resolved Hide resolved
src/utils/selector.ts Show resolved Hide resolved
@theerapatcha theerapatcha requested a review from Huachao July 8, 2020 14:16
@chazmuzz
Copy link

Shame this hasn't been merged, was there anything blocking it?

@abowen
Copy link

abowen commented Apr 1, 2022

Just coming from httpYac (https://httpyac.github.io/), which has this feature, any reason why this can't be merged?

@Huachao
Copy link
Owner

Huachao commented Apr 1, 2022

@theerapatcha the only block is that the code has conflicts, I will continue work on this when I am free

@theerapatcha
Copy link
Contributor Author

@Huachao resolved all the conflicts!

src/common/constants.ts Outdated Show resolved Hide resolved
src/utils/selector.ts Outdated Show resolved Hide resolved
@Huachao
Copy link
Owner

Huachao commented Apr 19, 2022

@theerapatcha I add some comments, and most of other parts looks really great to me

Utilize inputBox placeHolder for PromptVariable description
Fix excess grave accent (`) in http autocomplete element
@theerapatcha
Copy link
Contributor Author

@Huachao fixed as suggested (as well as one minor misplaced symbol). thanks!

@Huachao Huachao merged commit b55aa79 into Huachao:master Apr 19, 2022
@Huachao
Copy link
Owner

Huachao commented Apr 19, 2022

@theerapatcha Merged, thanks for your great contribution

@Huachao
Copy link
Owner

Huachao commented Apr 19, 2022

@chazmuzz @abowen @CaBazaga @jymannob @shuz with the great help from @theerapatcha , this feature has been merged and will be published in the next release.

@Huachao
Copy link
Owner

Huachao commented Jun 21, 2022

@theerapatcha @chazmuzz @abowen @CaBazaga @jymannob @shuz @unxia now you can try the Prompt variables in the latest version 0.25.0

@CaBazaga
Copy link

Great news, I just tested it and it works perfect. 🎊

If I may add a suggestion for improvement: Defaults.

Maybe taking any previously globally setted value for the given variable as a default (maybe prefilling the popup field) will work so in case I want to keep the default just hitting enter will work.

Something like

@someVar = SomeDefaultValue

### My request.
# @name MyRequest
# @prompt someVar SomeDescription
POST http://...

{
    "varName": "{{someVar}}"
}

So having the popup prefilled with the "SomeDefaultValue" allow me just to hit enter to accept it.

So there will be no need to add any extra syntax to achieve it, also if the text on the popup were preselected I can just type to overwrite the default or even just hit "backspace" to erase the popup field to input an "empty" value as it currently does.

Just a suggestion 😉

Good work anyway 👍

@chazmuzz
Copy link

Thank you so much!

@chazmuzz
Copy link

So having the popup prefilled with the "SomeDefaultValue" allow me just to hit enter to accept it.

Agree this would be awesome. Another suggestion in addition to defaults - remembering the previous value entered the last time I was prompted.

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.

Prompt for undefined variables
5 participants