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

Preprocessor: Add {LEFT_CURLY} and {RIGHT_CURLY} escape sequences #4432

Merged
merged 2 commits into from Oct 15, 2019

Conversation

@jostephd
Copy link
Member

commented Oct 5, 2019

See #4414

@CelticMinstrel

This comment has been minimized.

Copy link
Member

commented Oct 5, 2019

Though I think I'd prefer the names {LEFT_BRACE} and {RIGHT_BRACE}. Or if it's easy, maybe even support {[} and {]}.

@stevecotton

This comment has been minimized.

Copy link
Contributor

commented Oct 6, 2019

+1 for {LEFT_BRACE} and {RIGHT_BRACE}

I don't like {[}, it would be a pain to grep through the source code for.

@jostephd jostephd force-pushed the jostephd:preprocessor-add-left-curly branch from 74c8903 to f7fdb2a Oct 6, 2019
Copy link
Contributor

left a comment

I approve of the code changes, but I'm no longer sure if the feature is needed, so I'm in two minds about merging it. @CelticMinstrel 's suggestion to use _ << >> covers the needs of #4414.

@jostephd

This comment has been minimized.

Copy link
Member Author

commented Oct 8, 2019

Adding two lines of code for this seems worthwhile to me.

@CelticMinstrel

This comment has been minimized.

Copy link
Member

commented Oct 8, 2019

I feel like there could still be use-cases for this, though I'm not quite sure what they'd be... and it doesn't hurt to have two ways of doing the same thing.

Incidentally, I also just added a note on the wiki about the support for key=_<<>>.

@jostephd jostephd merged commit c0427b5 into wesnoth:master Oct 15, 2019
2 of 3 checks passed
2 of 3 checks passed
label
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jostephd added a commit that referenced this pull request Oct 15, 2019
@ProditorMagnus

This comment has been minimized.

Copy link
Contributor

commented Oct 15, 2019

These are not documented yet.

@jostephd

This comment has been minimized.

Copy link
Member Author

commented Oct 16, 2019

Thanks, fixed. I thought I'd forgotten something when I did yesterday's batch of merges...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.