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

Using dot (.) in variables names are recognized by the UI but not picked up when sending request #345

Closed
haneev opened this issue Oct 4, 2023 · 8 comments
Labels
bug Something isn't working good first issue Good for newcomers help wanted Community contribution is welcome

Comments

@haneev
Copy link

haneev commented Oct 4, 2023

Really great tool! Keep up the good work.

However I found a small bug πŸ™ƒ

Some variables/placeholders are not picked up by the engine (but are recognized by the UI).

I tested the following:

  • {{auth_token}} βœ…
  • {{auth-token}} βœ…
  • {{auth.token}} ⚠️

It does not work globally, so it does not work for url, body or headers.

@helloanoop
Copy link
Contributor

Some variables/placeholders are not picked up by the engine (but are recognized by the UI).

Yes, this needs to be fixed. Codemirror shouldn't be highlighting this variable. The . may lead to unpredictable behaviour since it may interfere with the handlebars templating logic

@helloanoop helloanoop added bug Something isn't working good first issue Good for newcomers help wanted Community contribution is welcome labels Oct 4, 2023
@lared
Copy link
Contributor

lared commented Oct 4, 2023

@helloanoop is it just dots? Do we have some sort of a regex for valid variable names?

@razzivofficial
Copy link

@helloanoop i tried to resolve this bug please review it if possible ! I am happy for any suggestions

@helloanoop
Copy link
Contributor

Hey @razzivofficial !

Current and Expected Behaviour

Currently - env vars, request and other vars don't have a very strict validation for naming present. The expectation is that they should follow javascript's variable naming validation which is

Variable Names Must Start with a Letter, Underscore (_), or Dollar Sign ($)

Unless there is a strong use-case (happy to change my mind if there is one) to name a var with other chars, bruno should use javascript's naming convention.

Nested Vars

Nested variables like {{auth.token}}

I have used the nested environments of insomnia in the past and like its benefits of have a base env and other env's extending the base env.

The reason why I went ahead with a simple key-val (like postman) was just because of its simplicity.

Is there a plan to support nested environments ?

Yes, In V2 - see roadmap
Issue: #292

Changes need to nudge users that the name is invalid

To solve this, We need to show an error when people try to save an environment with an invalid name.
@razzivofficial Can you work on making these changes here

I am assuming we can make the table cell color red for invalid names, and disable the save button

image

@martinsefcik
Copy link
Contributor

martinsefcik commented Oct 10, 2023

@helloanoop If you want to support some predefined/built-in variables in the future for example as Postman do (https://learning.postman.com/docs/writing-scripts/script-references/variables-list/) which start with $ I would not allow to define such variables with dollar character at the beginning by user to distinguish between them.

@bpoulaindev
Copy link
Contributor

@helloanoop I see variables using "." have been brought back, however this causes somme issue in the code generator tab. {{baseUrl}} works fine in both requests and code generator tabs, but {{base.url}} only works in requests. What's the current situation regarding these changes ?

@helloanoop
Copy link
Contributor

This whole issue around the need for . is basically happening because of the differences in how Postman and Insomnia have implemented env variable interpolation (Since a lot of users have used it in the past)

Postman has simple key value pairs, and Insomnia allows nested environments where . gets used

The reason why Bruno chose go with the simple key value approach was because of simplicity. Not everyone needs nested environments and forcing it by design on everyone didn't seem to be a good idea.

That said, we need to figure out a way to support nested environment as well as dot-notation based parsing. I don't have the answer right now, but we should be able to figure it out in the future and come up with an approach. For Ex A different syntax that parses dot notation: [[base.url]]

{{baseUrl}} works fine in both requests and code generator tabs, but {{base.url}} only works in requests. What's the current situation regarding these changes ?

@bpoulaindev We need to fix the code generator.

Also, I think the interpolation util is now used across 3 places - UI, Electron and CLI. We can move this into a single package bruno-common. We can also consolidate bruno-schema and bruno-query into this single package.

|- bruno-common
  | - src
    |- interpolation
    |- schema
    |- query

@Its-treason @martinsefcik thoughts ?

@helloanoop
Copy link
Contributor

The latest version of Bruno supports . in variable names.
Closing this issue.

If you are interested to see how Bruno handles interpolation, you can see the examples here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Community contribution is welcome
Projects
None yet
Development

No branches or pull requests

6 participants