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

Newline proposal #457

Open
max-sixty opened this issue Mar 5, 2024 · 3 comments
Open

Newline proposal #457

max-sixty opened this issue Mar 5, 2024 · 3 comments

Comments

@max-sixty
Copy link
Collaborator

max-sixty commented Mar 5, 2024

There's been some recent changes on how insta handles newlines. IIUC, we now just trim them from the start and end of every snapshot; this avoids showing incorrect diffs and inline snapshots failing to discriminate between newlines in the code and newlines in the snapshot.

I think there's an approach that would handle this well, but not confident and wanted to socialize it in case I'm missing something:

  • If an inline snapshot is sufficiently short1, it can go on one line; i.e.
    assert_snapshot!("foo", @"foo");
  • If an inline snapshot isn't sufficiently short, a newline gets added at the start and end which aren't part of the snapshot value; i.e.:
    assert_snapshot!(foo, @r#"
        multiple
        lines
    #");
    ...has a snapshot value of two lines; excluding the first and last newlines within the string
  • Then, if there are newlines in the snapshot value, these can then be recognized because we know we've added exactly one newline at the start and at the end. Here's an example if the snapshot value had a leading newline:
    assert_snapshot!(foo, @r#"
        
        multiple
        lines
    #");

This way, I don't think there can be any ambiguity.

Footnotes

  1. this generally means "is only one line", though very long single lines could start on the next line and become multiline

@max-sixty
Copy link
Collaborator Author

One issue I didn't consider above is that some autoformatters & editors attempt to ensure there's exactly one new line at the end of non-empty files (for example end-of-file-fixer). Some options:

  • Users should exclude .snap files from that check themselves
  • Finish with a YAML-like ---
  • Finish with some special character that isn't normally used, such as

@mitsuhiko
Copy link
Owner

mitsuhiko commented May 17, 2024

One option would also just be to make the start/end marker be a problem of the formatter and say that if this is relevant, we encourage/provide support to format blessed markers into the snapshot.

@max-sixty
Copy link
Collaborator Author

max-sixty commented Jun 17, 2024

#506 does the "Finish with a YAML-like ---" option.

We'd need to adjust the assertion code to check for newlines. And would also need a transition period where snapshots with different newlines pass with a warning that they won't in the future.

One option would also just be to make the start/end marker be a problem of the formatter and say that if this is relevant, we encourage/provide support to format blessed markers into the snapshot.

Is the "formatter" here yaml / json / ron? I think that could be fine but newlines are often an issue with blocks of text, which are likely using assert_snapshot. And we control yaml now!

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

No branches or pull requests

2 participants