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

Configure for the inline snapshot indentation #605

Closed
andylokandy opened this issue Sep 22, 2024 · 9 comments
Closed

Configure for the inline snapshot indentation #605

andylokandy opened this issue Sep 22, 2024 · 9 comments

Comments

@andylokandy
Copy link

Hi , thank you for the awesome tool!
Currently,cargo insta review generates inline snapshots in the form like:

assert_snapshot!(
    test.execute("select id, value from t1"),
    @r###"
+----+-------+
| id | value |
+----+-------+
| 2  | 102   |
+----+-------+
| 3  | 103   |
+----+-------+
| 4  | 104   |
+----+-------+

(3 rows)
"###
);

I wonder if it's possbile to make the following output with project-level configuration:

assert_snapshot!(
    test.execute("select id, value from t1"),
    @r###"
    +----+-------+
    | id | value |
    +----+-------+
    | 2  | 102   |
    +----+-------+
    | 3  | 103   |
    +----+-------+
    | 4  | 104   |
    +----+-------+

    (3 rows)
    "###
);

Maybe related to #457

@max-sixty
Copy link
Sponsor Collaborator

Thanks for the issue!

It should give reasonable indentation already. Could you post a reproducible example? The source can just be a literal string.

@andylokandy
Copy link
Author

Starting from this test:

#[test]
fn test_foo() {
    assert_snapshot!(
        "aaa\nbbb\nccc\nddd",
        @""
    );
}

After review:

#[test]
fn test_foo() {
    assert_snapshot!(
        "aaa\nbbb\nccc\nddd",
        @r###"
    aaa
    bbb
    ccc
    ddd
    "###
    );
}

If starting from oneline:

#[test]
fn test_foo() {
    assert_snapshot!("aaa\nbbb\nccc\nddd", @"");
}

After review:

#[test]
fn test_foo() {
    assert_snapshot!("aaa\nbbb\nccc\nddd", @r###"
    aaa
    bbb
    ccc
    ddd
    "###);
}

I've a tool using ast-grep to replace @ to - to make cargo fmt works (#499):

#[test]
fn test_foo() {
    assert_snapshot!("aaa\nbbb\nccc\nddd", -r###"
    aaa
    bbb
    ccc
    ddd
    "###);
}

cargo fmt:

#[test]
fn test_foo() {
    assert_snapshot!(
        "aaa\nbbb\nccc\nddd",
        -r###"
    aaa
    bbb
    ccc
    ddd
    "###
    );
}

replace - back to @:

#[test]
fn test_foo() {
    assert_snapshot!(
        "aaa\nbbb\nccc\nddd",
        @r###"
    aaa
    bbb
    ccc
    ddd
    "###
    );
}

@max-sixty
Copy link
Sponsor Collaborator

Sorry for being slow — am I understanding correctly that the current output and the desired output have the same indentation? And the difference is the opening delimiter?

(Am on phone so possibly I'm missing something...)

@andylokandy
Copy link
Author

Thank you for quick reply!
The desired output is

#[test]
fn test_foo() {
    assert_snapshot!(
        "aaa\nbbb\nccc\nddd",
        @r###"
        aaa
        bbb
        ccc
        ddd
        "###
    );
}

while I get

#[test]
fn test_foo() {
    assert_snapshot!(
        "aaa\nbbb\nccc\nddd",
        @r###"
    aaa
    bbb
    ccc
    ddd
    "###
    );
}

@max-sixty
Copy link
Sponsor Collaborator

I think a couple of things here:

  • Specifically the indentation: this is selected at the same level as the macro. Which I think generally works OK? But open to alternatives.
    • Though I'd be very hesitant to add a configuration, we want to have a simple interface, so we should come up with a rule that works across most cases. Is there a better rule than the current one?
  • Formatting: I agree the formatting is occasionally not be ideal. cargo fmt works within macros iff the macro contents is valid rust, which @ isn't.
    • Ideally we could have syntax valid rust syntax, and then cargo fmt would work. Currently the difficulty is that we need to distinguish between assert_snapshot!(name, expr) and assert_snapshot!(expr, inline_value), and so we need something that isn't a valid rust expression. So I can't think of a way around it
    • Your ast-grep approach is clever!

@andylokandy
Copy link
Author

andylokandy commented Sep 22, 2024

Indeed. These maybe project-specific preference. The better way could be keeping the default behavior and format using ast-grep in project desiring different style.

Thank you very much!

@andylokandy
Copy link
Author

andylokandy commented Sep 22, 2024

so we need something that isn't a valid rust expression. So I can't think of a way around it

rustfmt is requiring a syntactical valid expression, but not semantically valid expression. Thus there is room to find such patterns. For example, my using -r###""### utilize the fact that unary minus to a string is valid in syntax but invalid in type. Symbols like *, !, ?(postfix) are also possible.

Maybe further, in assert_snapshot!(expr, string_lit), we can always treat it as inline snap if the second argument is string literal because it's not likely assert_snapshot!(name, expr) has a simple string literal as expr (comparing a string lit with string lit).

@max-sixty
Copy link
Sponsor Collaborator

(great, will reply in #499)

@max-sixty
Copy link
Sponsor Collaborator

To reply to:

Maybe further, in assert_snapshot!(expr, string_lit), we can always treat it as inline snap if the second argument is string literal because it's not likely assert_snapshot!(name, expr) has a simple string literal as expr (comparing a string lit with string lit).

I would vote to stay away from any ambiguities like this; confusion there would be very expensive...

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