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

Yaml - coalesce less #1841

Open
nmck257 opened this issue May 24, 2022 · 0 comments
Open

Yaml - coalesce less #1841

nmck257 opened this issue May 24, 2022 · 0 comments
Labels
bug Something isn't working

Comments

@nmck257
Copy link
Collaborator

nmck257 commented May 24, 2022

As discussed w/ @jkschneider on slack, consider a test case like this:

@Test
fun `does not reformat properties outside the affected key`() = rewriteRun({ spec ->
        spec.recipe(ChangePropertyKey("a.b.c", "x.y.z", true, null))
    },
    yaml(
        """
        a:
          b:
            c: abc
        something:
          else: qwe
        """, """
        x:
          y:
            z: abc
        something:
          else: qwe
        """
    )
)

This case fails, as the actual value is this:

# file leads with an empty line
something.else: qwe
x.y.z: abc

Few observations, ordered subjectively from most-to-least severe:

  • The key something.else gets coalesced, though it was unrelated to the change
  • x.y.z gets coalesced, though the key it replaced was styled as non-coalesced
  • x.y.z gets relocated in the file
  • The resultant file has an extraneous newline at the beginning

These tend to conflict w/ this design intent: "OpenRewrite recipes make minimally invasive changes to your source code that honor the original formatting" (from the main page of the docs site).

This issue targets that first observation, and we'd like to try simply removing nested usages of CoalesceProperties from other recipes. This should get us closer to a target state, with low risk of functional changes.

This could include:

  • ChangePropertyKey
  • DeleteProperty
  • MergeBootstrapYamlWithApplicationYaml
  • MigrateDatabaseCredentials

Future enhancements could include defining Styles for Yaml, to better-protect indent-vs-dot preferences.

@pway99 pway99 added the bug Something isn't working label May 25, 2022
@pway99 pway99 moved this to Backlog in OpenRewrite May 25, 2022
nmck257 added a commit to nmck257/rewrite that referenced this issue May 31, 2022
nmck257 added a commit to nmck257/rewrite that referenced this issue Jun 1, 2022
nmck257 added a commit to nmck257/rewrite that referenced this issue Jun 1, 2022
pway99 pushed a commit that referenced this issue Jun 1, 2022
* Removed coalescing from Yaml ChangePropertyKey, and deprecated coalescing in DeleteProperty and YamlVisitor (#1841)

* Fixed edge case where DeleteProperty left an extraneous newline if deleting the first element in a document (#1841)
pway99 added a commit that referenced this issue Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Backlog
Development

No branches or pull requests

2 participants