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

Implement the private attribute for assignments #2300

Merged
merged 8 commits into from
Aug 10, 2024

Conversation

adsnaider
Copy link
Contributor

This PR fixes #2299. When a variable is specified as [private] or is named with _* the variable will be hidden from just --variables and forbidden from being overridden. I purposefully didn't change the behavior of the evaluator on private variables since it's likely useful to be able to view the values of private variables for debugging & introspection.

Copy link
Owner

@casey casey left a comment

Choose a reason for hiding this comment

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

The [private] attribute on recipes doesn't prevent running private them, only listing them, and I'm inclined to think the [private] variable attribute should similar, i.e., only prevent listing them, but still allow overriding them.

@casey
Copy link
Owner

casey commented Aug 4, 2024

I would also filter them out of --evaluate. We can think about some kind of --all flag in a follow-up PR, which un-hides them. (--all should probably also allow listing all recipes, including private recipes)

@adsnaider
Copy link
Contributor Author

I believe this is ready again. Haven't implemented the --all flag but the other changes should now be working properly and as you point out that flag can be added later

Copy link
Owner

@casey casey left a comment

Choose a reason for hiding this comment

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

Sorry I missed this the first time! I would remove the allow-private-variables setting, since I think an --all flag, which can be added later, is more useful and general.

@adsnaider
Copy link
Contributor Author

Ah sure, I assumed that would be a backwards-incompatible change

@laniakea64
Copy link
Contributor

Ah sure, I assumed that would be a backwards-incompatible change

Using the [private] attribute on variables is backwards-compatible, since it's an error to do that in current just, so it won't change behavior of any working justfiles. So no need for a setting for that.

But wouldn't making all underscore-prefixed variables private be backwards-incompatible, and therefore that aspect would need an opt-in setting?

@adsnaider
Copy link
Contributor Author

@laniakea64 Yeah, that's what I thought. I could take out the _ prefix for private variables 🤷

@casey
Copy link
Owner

casey commented Aug 5, 2024

It could be argued either way, but I think making variables private if they start with _ is probably fine. It won't actually break any justfiles, just remove those variables from the output of just --evaluate and just --variables, which are, I think, mostly for human consumption.

@adsnaider
Copy link
Contributor Author

Sorry I missed this the first time! I would remove the allow-private-variables setting, since I think an --all flag, which can be added later, is more useful and general.

Removed the setting @casey

Copy link
Owner

@casey casey left a comment

Choose a reason for hiding this comment

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

LGTM!

@casey casey enabled auto-merge (squash) August 10, 2024 18:20
@casey casey merged commit f25e2d7 into casey:master Aug 10, 2024
5 checks passed
@adsnaider adsnaider deleted the private_assignment branch August 10, 2024 18:22
@casey
Copy link
Owner

casey commented Aug 28, 2024

Available in 1.35.0, just released: https://github.com/casey/just/releases/tag/1.35.0

Jasha10 added a commit to Jasha10/just that referenced this pull request Aug 29, 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.

Allow defining private variables
3 participants