Skip to content

Conversation

@jmgomez
Copy link
Collaborator

@jmgomez jmgomez commented Aug 28, 2025

No description provided.

proc eqIdent(a, b: string): bool {.inline.} =
cmpIgnoreCase(a, b) == 0 and a[0] == b[0]

proc compileDefines(): Table[string, bool] =
Copy link
Member

@Araq Araq Aug 28, 2025

Choose a reason for hiding this comment

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

Use std / strtabs instead which supports cmpIgnoreStyle.

@arnetheduck
Copy link

Overall, this approach represents a direction that we might ultimately not want in the future, ie whose details will likely change - backwards compatibility with packages that use when for now is already served by the fallback mode (which itself has some improvements scheduled) so there is no urgency to introduce partial compatibility features based on hacks like compileDefines just for the sake of enabling the parsing of a few more packages to use the new parser.

It's probably reasonable to keep this PR in draft until things have settled around other in-development things like feature flag set arithmetic - once that is done, we can revisit the "language" for conditional requirements and its interaction with the system / nim etc

@Araq
Copy link
Member

Araq commented Aug 28, 2025

Atlas uses this approach successfully (and it doesn't offer a fallback at all). It's reasonably easy to make it sound, document the available defines and the fact that you have the not/and/or boolean operators. Produce an error for an unsupported define. It's not a hack, it merely started as one...

@jmgomez jmgomez marked this pull request as draft August 29, 2025 09:01
@arnetheduck
Copy link

reasonably easy to make it sound

thanks to the fallback in nimble, we can make it sound first, then add it as a feature, so that we don't have to maintain backwards compatibility with a potentially unsound or incomplete version in the new code) - ie apart from soundness, it would be good to have completeness as well in the same package and consider other formats for it (requires(..., cond="win or mac" for example before committing) once feature flags reach maturity since the two things are codependent, UX-wise.

@jmgomez jmgomez mentioned this pull request Sep 17, 2025
19 tasks
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.

3 participants