Skip to content

[Dotenv] Variable filter added to debug command #46502

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.

Sign up for GitHub

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

Jump to bottom
Merged
fabpot merged 1 commit into from
Jul 20, 2022
Merged

[Dotenv] Variable filter added to debug command #46502

fabpot merged 1 commit into from
Jul 20, 2022

Conversation

@rmikalkenas
Copy link
Contributor

@rmikalkenas rmikalkenas commented May 30, 2022 •
edited
Loading

Q A
Branch? 6.2
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR Will prepare on implementation approval

For applications that use many environment variables there is no way to efficiently filter a single variable. Of course there is always a grep command, but with it you are loosing part of the information that symfony debug command provides.
Added a new name argument to the command, to be able to filter debug output by variable's name

@carsonbot carsonbot added this to the 6.2 milestone May 30, 2022
GromNaN
GromNaN reviewed May 30, 2022
View reviewed changes
Copy link
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

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

I like this feature. It's consistent with debug:container and debug:router that allows filtering with an optional argument.

94noni
94noni reviewed May 30, 2022
View reviewed changes
@rmikalkenas rmikalkenas force-pushed the ability-debug-single-env branch 2 times, most recently from 973fee1 to f501856 Compare May 30, 2022 17:45
@rmikalkenas
Copy link
Contributor Author

rmikalkenas commented May 30, 2022

@GromNaN @94noni thanks for the comments, adjusted the code based on it

@rmikalkenas rmikalkenas force-pushed the ability-debug-single-env branch from f501856 to 89f34a7 Compare May 30, 2022 18:02
@rmikalkenas
Copy link
Contributor Author

rmikalkenas commented May 30, 2022

P.S. added filtering not only by exact value, but by prefix as well. For example if there are more than one variable with same prefix in the name - it will be possible to filter by prefix and get all values with that prefix:)

GromNaN
GromNaN requested changes May 31, 2022
View reviewed changes
Copy link
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

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

Thanks for the completion and tests.

I tested the command and I think a message should be displayed when none of the variables starts with the given name.

@rmikalkenas rmikalkenas force-pushed the ability-debug-single-env branch from 89f34a7 to bf4036e Compare June 1, 2022 12:39
GromNaN
GromNaN approved these changes Jun 1, 2022
View reviewed changes
@rmikalkenas rmikalkenas force-pushed the ability-debug-single-env branch from bf4036e to c3f09e6 Compare June 1, 2022 13:52
fabpot
fabpot requested changes Jun 2, 2022
View reviewed changes
@rmikalkenas rmikalkenas force-pushed the ability-debug-single-env branch 2 times, most recently from af5e53c to 6e6cd83 Compare June 3, 2022 08:39
@rmikalkenas rmikalkenas requested a review from fabpot June 3, 2022 08:45
@rmikalkenas
Copy link
Contributor Author

rmikalkenas commented Jun 3, 2022 •
edited
Loading

@fabpot updated, based on comments. The failing pipeline does not relate with my changes (except for the low-deps of 8.1 which is due to dependency of 5.4 symfony components)

@rmikalkenas rmikalkenas force-pushed the ability-debug-single-env branch from 6e6cd83 to d41e7a1 Compare June 23, 2022 12:32
GromNaN
GromNaN reviewed Jul 14, 2022
View reviewed changes
Copy link
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

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

I'd be glad if this feature was approved.

fabpot
fabpot approved these changes Jul 15, 2022
View reviewed changes
Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

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

2 small changes before we can merge.

src/Symfony/Component/Dotenv/Command/DebugCommand.php Outdated
<info>php %command.full_name%</info>
To get specific variable, specify its name:
Copy link
Member

@fabpot fabpot Jul 15, 2022

Choose a reason for hiding this comment

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

So, it can be the env name or part of the name, right?

Copy link
Contributor Author

@rmikalkenas rmikalkenas Jul 18, 2022

Choose a reason for hiding this comment

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

Updated with clarification: To get specific variable, specify its full or partial name:

@rmikalkenas rmikalkenas force-pushed the ability-debug-single-env branch from d41e7a1 to a754669 Compare July 18, 2022 12:26
@fabpot
Copy link
Member

fabpot commented Jul 20, 2022

Thank you @rmikalkenas.

@fabpot fabpot mentioned this pull request Jul 20, 2022
Closed
@fabpot fabpot merged commit ca4615a into symfony:6.2 Jul 20, 2022
@rmikalkenas rmikalkenas deleted the ability-debug-single-env branch July 20, 2022 10:38
@chalasr
Copy link
Member

chalasr commented Jul 20, 2022 •
edited
Loading

@rmikalkenas The testCompletion test added by this PR is failing under certain circumstances. Would you mind having a look?
To reproduce locally:
./phpunit src/Symfony/Component/Dotenv --filter testCompletion (all green when running the whole DebugCommandTest 🤷 )

@rmikalkenas
Copy link
Contributor Author

rmikalkenas commented Jul 21, 2022

@rmikalkenas The testCompletion test added by this PR is failing under certain circumstances. Would you mind having a look? To reproduce locally: ./phpunit src/Symfony/Component/Dotenv --filter testCompletion (all green when running the whole DebugCommandTest shrug )

@chalasr thanks for informing! Sounds weird.. Will take a look!

@rmikalkenas rmikalkenas mentioned this pull request Jul 21, 2022
Merged
fabpot added a commit that referenced this pull request Jul 22, 2022
@fabpot
minor #47014 [Dotenv] Run test in a separate process to have clean gl…
8be4d28 
…obal state (rmikalkenas)

This PR was merged into the 6.2 branch.

Discussion
----------

[Dotenv] Run test in a separate process to have clean global state

| Q             | A
| ------------- | ---
| Branch?       | 6.2
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #...
| License       | MIT
| Doc PR        | symfony/symfony-docs#...

A fix to comment: #46502 (comment)
cc @chalasr

Commits
-------

be3782c [Dotenv] Run test in a separate process to have clean global state
@fabpot fabpot mentioned this pull request Oct 24, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@GromNaN GromNaN GromNaN approved these changes

@fabpot fabpot fabpot approved these changes

+1 more reviewer

@94noni 94noni 94noni approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Dotenv Feature Status: Reviewed

Projects

None yet

Milestone

6.2

Development

Successfully merging this pull request may close these issues.

6 participants

@rmikalkenas @fabpot @chalasr @GromNaN @94noni @carsonbot