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

Spaces are incorrectly added before indented parameters while formatting a fragment #1738

Closed
vinocher-bc opened this issue Jun 20, 2024 · 8 comments
Labels
bug fixed in develop bug/feature resolved in the develop branch

Comments

@vinocher-bc
Copy link

vinocher-bc commented Jun 20, 2024

Spaces are incorrectly added before indented parameters while formatting the parameters of a function call in a fragment. This was reproduced on Windows.

Repro:
(Use Ctrl+Z, Enter to signal EOF in stdin):
Note the spaces added before foo1 & foo2

C:\>swiftformat --fragment true stdin
   foo: bar,
   foo1: bar2,
   foo2: bar3
^Z
Running SwiftFormat...
   foo: bar,
       foo1: bar2,
       foo2: bar3
SwiftFormat completed successfully.

The formatting of the indentation is correct if the entire function call is provided:

C:\>swiftformat --fragment true stdin
myFunc(
   foo: bar,
   foo1: bar2,
   foo2: bar3
)
^Z
Running SwiftFormat...
myFunc(
    foo: bar,
    foo1: bar2,
    foo2: bar3
)
SwiftFormat completed successfully.
@calda
Copy link
Collaborator

calda commented Jun 20, 2024

I believe SwiftFormat expects all input to be syntactically-valid Swift code, and the behavior of rules is more of less undefined when passing in invalid Swift code.

Since this input code isn't valid Swift code, to me it isn't unexpected that the output would be unusual:

   foo: bar,
   foo1: bar2,
   foo2: bar3

Do you have a repro case for this where the indentation is incorrect when passing in text that is valid Swift code?

@vinocher-bc
Copy link
Author

vinocher-bc commented Jun 20, 2024

@calda This fragment is part of valid Swift Code, and the expectation is that since --fragment true is passed, SwiftFormat will be conservative in its formatting and keep the original indentation. Is that expectation correct? Thanks.

This scenario can occur in editors like VSCode which have a "editor.formatOnSaveMode": "modifications" setting to format only code changes -- see Only Format Modified Text. Such a setting prevents unnecessary diffs when a document is saved, by formatting only the new changes.

In this case, the following parameters are part of valid Swift Code. For example, assume that a user made changes to the parameters of an existing function call in a document. In that case, a fragment containing the parameters will need to be formatted.

Fragment:

   foo: bar,
   foo1: bar2,
   foo2: bar3

Context:

  myFunc(
    foo: bar,
    foo1: bar2,
    foo2: bar3
  )

@calda calda removed the not a bug label Jun 20, 2024
@vinocher-bc vinocher-bc changed the title Spaces are incorrectly added before indented parameters Spaces are incorrectly added before indented parameters while formatting a fragment Jun 20, 2024
@nicklockwood
Copy link
Owner

@calda yeah @vinocher-bc is correct. I think the simplest fix here is just to disable the wrap rule when running in --fragment mode (there are probably many other rules that also should be disabled but currently aren't)

@nicklockwood
Copy link
Owner

@vinocher-bc that said, I think the vs-code extension should not be using --fragment. It was originally introduced for this purpose as part of the SwiftFormat Xcode extension, but I replaced it with --linerange instead, which is much more reliable because it takes the entire file into account for parsing but limits any changes to just the selected region.

@nicklockwood nicklockwood added bug fixed in develop bug/feature resolved in the develop branch labels Jun 23, 2024
@vinocher-bc
Copy link
Author

@vinocher-bc that said, I think the vs-code extension should not be using --fragment. It was originally introduced for this purpose as part of the SwiftFormat Xcode extension, but I replaced it with --linerange instead, which is much more reliable because it takes the entire file into account for parsing but limits any changes to just the selected region.

@nicklockwood I'm seeing some weirdness with --linerange formatting ,where the output indentation is incorrect:

C:\>swiftformat --linerange 3, 4 stdin
myFunc(
    foo: bar,
    foo1: bar1,
        foo2: bar2,
        foo3: bar3
)
^Z
Running SwiftFormat...
myFunc(
    foo: bar,
    foo1: bar1,
        foo2: bar2,
    foo3: bar3
)
SwiftFormat completed successfully.

@vinocher-bc
Copy link
Author

vinocher-bc commented Jun 24, 2024

@calda a similar issue occurs with method chaining. The indentation is removed. Expected: the indentation should not be removed.
The behavior seems worse with --linerange. If needed, I can open another issue to track this. Thanks.

 C:\>swiftformat --fragment true stdin
    .foo ( )
^Z
Running SwiftFormat...
.foo()
SwiftFormat completed successfully.
C:\>swiftformat --linerange 0,0 stdin
   .foo ( )
^Z
Running SwiftFormat...
.foo ( )
SwiftFormat completed successfully.

It is formatted correctly when the context is provided:

C:\>swiftformat --fragment true stdin
    baz
      .foo  ( )
      .bar (    )
^Z
Running SwiftFormat...
    baz
        .foo()
        .bar()
SwiftFormat completed successfully.

@nicklockwood
Copy link
Owner

@vinocher-bc --linerange assumes the entire file is passed as input, but only applies changes to the specified range. If you are passing only a fragment of the file then --fragment is the correct argument to use

@nicklockwood
Copy link
Owner

@vinocher-bc --fragment support for indent is fixed in 0.54.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fixed in develop bug/feature resolved in the develop branch
Projects
None yet
Development

No branches or pull requests

3 participants