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

Support single delimiter version of expect![] #27

Merged
merged 6 commits into from
Mar 4, 2022

Conversation

ecstatic-morse
Copy link
Contributor

Pairs of braces are no longer necessary that we parse string literals. Allow users to omit the inner [].

This represents a significant change to the public API, and you might have other tooling that depends on the paired braces, so I understand if you don't want it upstream. I think it reads a bit nicer, though.

Depends on #26 (although it could be separated), and is similarly lacking in integration tests for now.

@ecstatic-morse ecstatic-morse force-pushed the braceless branch 3 times, most recently from 320ba76 to 81e1333 Compare December 26, 2021 22:21
@matklad
Copy link
Member

matklad commented Dec 27, 2021

The primary motivation for [[]] is to hack around rustfmt suboptimal formatting. With one set of brackets, it adds two extra newlines per expect macro.

@ecstatic-morse
Copy link
Contributor Author

The primary motivation for [[]] is to hack around rustfmt suboptimal formatting. With one set of brackets, it adds two extra newlines per expect macro.

Ah, I didn't think of that. Worth noting that this isn't true for single-line expects, for which the terser syntax is most impactful.

@ecstatic-morse ecstatic-morse marked this pull request as draft December 27, 2021 19:10
@matklad
Copy link
Member

matklad commented Jan 3, 2022

Yeah, so let's do this:

  • support one or two delimiters, but not more
  • use one-delimited simple string for single-line cases
  • do not preserve user spelling. Ie, if the user writes expect![[r#"foo"#]], and we update this to bar, we don't take the original form into account and use expect!["bar"]. Not that this is for the cases where we do update. If the test passes, it should pass with whatever flavor of delimiters.

@ecstatic-morse
Copy link
Contributor Author

ecstatic-morse commented Jan 4, 2022

do not preserve user spelling. Ie, if the user writes expect![[r#"foo"#]], and we update this to bar, we don't take the original form into account and use expect!["bar"]. Not that this is for the cases where we do update. If the test passes, it should pass with whatever flavor of delimiters.

This is implemented now (it requires #28). It does add some complexity, however. Let me know what you think.

@ecstatic-morse ecstatic-morse marked this pull request as ready for review January 5, 2022 01:18
@ecstatic-morse
Copy link
Contributor Author

ecstatic-morse commented Feb 4, 2022

Ping @matklad, in case this got lost during the holidays. Seems like chalk will start using expect-test soon-ish, and some form of this would be nice to have.

@matklad
Copy link
Member

matklad commented Mar 4, 2022

Sorry, this felt through the cracks, it looks good to me now!

bors r+

@bors
Copy link
Contributor

bors bot commented Mar 4, 2022

Build succeeded:

@bors bors bot merged commit ec83506 into rust-analyzer:master Mar 4, 2022
@Veetaha Veetaha mentioned this pull request May 24, 2022
@xxchan xxchan mentioned this pull request May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants