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

Allow indentation width to be set explicitly #8

Merged
merged 1 commit into from
Nov 15, 2023

Conversation

gohanlon
Copy link
Contributor

@gohanlon gohanlon commented Oct 6, 2023

This allows the user to circumvent the default indentation behavior. In particular, it can be used to avoid getting the default four-space indentation when expanding source that doesn’t contain any indentation to infer the indentation width from, e.g.:

@AddMember
struct S {
}

Unindented original source seems uncommon, but it occurs pretty frequently in my macro tests. For example, I have a collection of tests that exercise each possible combination of access modifier for structs, classes, and actors.

Changes:

  • The assertMacro and withMacroTesting functions now accept indentationWidth as a parameter (Trivia).
  • Introduces IndentationWidthTests to validate both the existing indentation behavior and the new explicit indentation width functionality.

Some alternatives to consider:

  • Instead offer defaultIndentationWidth to let the user only control the fallback indentation value. This would work in my use case.
  • Only support explicit indentation width in assertMacro and not in withMacroTesting.

This allows the user to circumvent the default indentation behavior. In
particular, it can be used to avoid getting the default four-space
indentation when expanding source that doesn’t contain any indentation
to infer the indentation width from.

Changes:

* The `assertMacro` function, along with related functions, now accepts
  `indentationWidth` as a parameter (`Trivia`).
* Introduces `IndentationWidthTests` to validate both the existing
  indentation behavior and the new explicit indentation width
  functionality.
@@ -90,8 +91,11 @@ import XCTest
///
/// - Parameters:
/// - macros: The macros to expand in the original source string. Required, either implicitly via
/// ``withMacroTesting(isRecording:macros:operation:)-2vypn``, or explicitly via this parameter.
/// no fix-its to apply, or if any diagnostics are unfixable, the assertion will fail.
Copy link
Contributor Author

@gohanlon gohanlon Oct 7, 2023

Choose a reason for hiding this comment

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

I removed this stray line:

///      no fix-its to apply, or if any diagnostics are unfixable, the assertion will fail.

which was left behind when applyFixIts was removed:

///
///   - applyFixIts: Applies fix-its to the original source and snapshots the result. If there are
///     no fix-its to apply, or if any diagnostics are unfixable, the assertion will fail.
///

Copy link
Member

@stephencelis stephencelis left a comment

Choose a reason for hiding this comment

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

Thanks for diving into this! Brandon and I will chat about it early next week.

@gohanlon
Copy link
Contributor Author

Hi @stephencelis, any updates on this? A way to clean up the awkward indentation in a dozen or so of my tests would be a nice-to-have! 😄

Copy link
Member

@stephencelis stephencelis left a comment

Choose a reason for hiding this comment

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

@gohanlon Sorry, we just lost track of this. Thanks!

@stephencelis stephencelis merged commit 6e93798 into pointfreeco:main Nov 15, 2023
2 checks passed
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.

2 participants