microsoft / vscode-cpptools

Official repository for the Microsoft C/C++ extension for VS Code.
Other
5.51k stars 1.55k forks source link

"Format Selection" will use style based of preceding function instead of `C_Cpp:Clang_format_style` #10839

Open cj-obrien opened 1 year ago

cj-obrien commented 1 year ago

Environment

Bug Summary and Steps to Reproduce

Bug Summary:

I have set C_Cpp:Clang_format_style to:

{ BasedOnStyle: LLVM, UseTab: Never, IndentWidth: 2, TabWidth: 2, SortIncludes: Never }.

However, when I select a block of code that use a different number of spaces for indentation (four, for example) and select Format Selection from the command palette, the indentation does not change.

Steps to reproduce:

  1. Select Preferences: Open Settings (UI) from the command palette
  2. Click on 'User'
  3. Enter C_Cpp:Clang_format_style in the search box
  4. Paste { BasedOnStyle: LLVM, UseTab: Never, IndentWidth: 2, TabWidth: 2, SortIncludes: Never } into the text box
  5. Open C++ source file
  6. Select method using four spaces for indentation
  7. Select Format Selection from the command palette
  8. Observe that indentation of selected block does not change

Expected behavior:

Indentation of selected block does changes from four spaces to two spaces.

Configuration and Logs

LSP: textDocument/hover: file:///<redacted>.cpp (id: 2495)
LSP: cpptools/textEditorSelectionChange
LSP: cpptools/textEditorSelectionChange
LSP: cpptools/textEditorSelectionChange
LSP: cpptools/textEditorSelectionChange
LSP: cpptools/textEditorSelectionChange
LSP: cpptools/textEditorSelectionChange
LSP: cpptools/textEditorSelectionChange
LSP: cpptools/textEditorSelectionChange
LSP: cpptools/getCodeActions: file:///<redacted>.cpp (id: 2496)
LSP: cpptools/formatRange: file:///<redacted>.cpp (id: 2497)
Formatting document: file:///<redacted>.cpp
Formatting Engine: clangFormat

Other Extensions

No response

Additional context

No response

michelleangela commented 1 year ago

@cj-obrien I couldn't repro the issue. Could you check if formatting works for other cases? For example, what happens when "Format Document" is done? Could you also provide the Clang format version you're using?

The workspace settings I used were:

"C_Cpp.clang_format_style": "{ BasedOnStyle: LLVM, UseTab: Never, IndentWidth: 2, TabWidth: 2, SortIncludes: Never }",
"C_Cpp.formatting": "clangFormat"

Sample code used:

#include <string>

void myfunction()
{
    std::string text = "abc";
    text.append("123");
}

Extension version 1.14.5 Clang-format version 15.0.3

Repro steps:

  1. Select lines std::string text = "abc"; and text.append("123");
  2. Right click on selected lines and run "Format Selection"

Result: the indentation changed from 4 spaces to 2 spaces.

cj-obrien commented 1 year ago

@michelleangela At least part of the problem appears to be missing double-quotes around the string in the C_Cpp:Clang_format_style field. When I added the missing '"' characters (the only difference between your setting and mine), the debug logs in the Output pane show the following:

Formatting Engine: clangFormat
Formatting failed:
/home/vscode/.vscode-server/extensions/ms-vscode.cpptools-1.14.5-linux-x64/bin/../LLVM/bin/clang-format '-style="{ basedonstyle: llvm, usetab: never, indentwidth: 3, tabwidth: 3, sortincludes: never }"' -fallback-style=LLVM --Wno-error=unknown -offset=19 -length=76 -assume-filename=/workspaces/<redacted>/foo.cpp

Without the quotes, I do not see the "Formatting failed" message.

sean-mcmanus commented 1 year ago

@cj-obrien I believe the extra " are actually causing clang-format to fail. The quotes referenced by "C_Cpp.clang_format_style": "{ BasedOnStyle: LLVM, UseTab: Never, IndentWidth: 2, TabWidth: 2, SortIncludes: Never }", are required for the JSON file and are not passed to clang-format.

cj-obrien commented 1 year ago

Thanks @sean-mcmanus. I infer then that the following log entry:

Formatting document: file:///<redacted>.cpp
Formatting Engine: clangFormat

indicates that formatting was completed successfully?

Interestingly, when I create a new file in my existing project, paste the lines from @michelleangela's example above into it, select myfunction() and then issue FormatSelection, the indentation changes from four spaces to two. When I pick another (existing) source file from the project, select a method and issue FormatSelection, some formatting is applied, but the indentation does not change.

However, FormatDocument does change the indentation correctly.

cj-obrien commented 1 year ago

@michelleangela Try the following sample code:

#include <string>

void myfunction() {
    std::string text = "abc";
    text.append("123");
}

void my_other_function() {
    std::string text = "cba";
    text.append("321");
}

When I paste this into a file, I'm able to select myfunction() and use FormatSelection to change the indentation, but the same process does not work when my_other_function() is selected.

cj-obrien commented 1 year ago

Another finding: formatting myfunction(), followed by my_other_function() works. If I reverse the order only the indentation of myfunction() is modified.

michelleangela commented 1 year ago

@cj-obrien I was able to reproduce the issue when trying to format any function that is defined after a function that does not yet have the desired format style.

The "Format Selection" will use the preceding function's format style as the style to apply to the selected function, instead of using the style defined in C_Cpp.clang_format_style. The style in C_Cpp.clang_format_style gets applied to a whole document format. But when it comes to formatting a selection, the style that is used is based on a preceding function's style (even if the style is incorrect by intention for testing purposes). If there are no preceding functions, then the style from C_Cpp.clang_format_style is used.

I think it is expected that "Format Selection" should use the style defined in C_Cpp.clang_format_style instead of inheriting a style from a preceding function.

We'll investigate on how the extension handles "Format Selection".

Repro example:

#include <string>

void myfunction() {
std::string text = "abc";
text.append("123");
}

void my_other_function() {
            std::string text = "cba";
            text.append("321");
}

void my_last_function() {
    std::string text = "cba";
    text.append("321");
}
  1. First, format my_last_function() via "Format Selection". This will use style based of my_other_function().
  2. Format myfunction() via "Format Selection". This will use style defined in C_Cpp.clang_format_style.
  3. Format my_other_function() last via "Format Selection". This will use style based of myfunction().