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

Enforce only valid rule variables are used #96

Closed
wants to merge 1 commit into from

Conversation

Colecf
Copy link
Contributor

@Colecf Colecf commented Jan 3, 2024

Regular ninja also has this validation.

Regular ninja also has this validation.
@evmar
Copy link
Owner

evmar commented Jan 8, 2024

I think my just-filed #99 is relevant here: when authoring a tool that generates ninja files this kind of check is useful, but otherwise there's not a much to be gained by being more strict.

@Colecf
Copy link
Contributor Author

Colecf commented Jan 8, 2024

Are you saying you want this to be behind a strict flag for performance reasons? Aside from performance I don't think there's any downsides to adding this, seeing as everyone's already complying with it due to the fact ninja has it unconditionally.

I haven't benchmarked the performance, but it seems like it should be extremely minor, and not worth potentially making people's lives harder if they started without strict mode and then tried to adopt it later, but had already took advantage of the flexibility.

@evmar
Copy link
Owner

evmar commented Jan 18, 2024

I think I merged this as part of merging the other one, lemme know if it needs rebasing

@Colecf
Copy link
Contributor Author

Colecf commented Jan 18, 2024

Yes, sorry, I don't really know what I'm doing with github pull requests based on top of each other.

@Colecf Colecf closed this Jan 18, 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.

2 participants