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.

Sign up for GitHub

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

Passing extra formatting options to LSPs #2635

Merged
merged 4 commits into from
Jun 5, 2022
Merged

Passing extra formatting options to LSPs #2635

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
644a1f5
allows passing extra formatting options to LSPs
farwyler Jun 1, 2022
563fccf
cleaner conversion of formatting properties
farwyler Jun 1, 2022
6c29b0e
move formatting options inside lsp::Client
farwyler Jun 2, 2022
7c7779b
cleans up formatting properties merge
farwyler Jun 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions book/src/languages.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,18 @@ name = "rust"
auto-format = false
```

## LSP formatting options

Use `format` field to pass extra formatting options to [Document Formatting Requests](https://github.com/microsoft/language-server-protocol/blob/gh-pages/_specifications/specification-3-16.md#document-formatting-request--leftwards_arrow_with_hook).

```toml
[[language]]
name = "typescript"
auto-format = true
# pass format options according to https://github.com/typescript-language-server/typescript-language-server#workspacedidchangeconfiguration omitting the "[language].format." prefix.
format = { "semicolons" = "insert", "insertSpaceBeforeFunctionParenthesis" = true }
Copy link
Member

Choose a reason for hiding this comment

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

This was a mistake we did with language server configuration: I'd prefer format was nested under the language-server key. Though maybe we can keep it toplevel

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would make sense inside the language server configuration, especially if we ever plan to support multiple LS per language. I have seen neovim setups where multiple LS get attached to a buffer, mostly for chaining formatting functionality. Can't say that i liked it. Have you guys thought in that direction? I haven't noticed the topic here, but i am just getting into the project.

Copy link
Member

Choose a reason for hiding this comment

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

#1396

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i guess we need this. 6c29b0e nests it inside the lsp config now.

```

## Tree-sitter grammars

Tree-sitter grammars can also be configured in `languages.toml`:
Expand Down
3 changes: 3 additions & 0 deletions helix-core/src/syntax.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ pub struct LanguageConfiguration {

#[serde(default)]
pub auto_format: bool,
#[serde(default, skip_serializing, deserialize_with = "deserialize_lsp_config")]
pub format: Option<serde_json::Value>,
Copy link
Member

@archseer archseer Jun 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, why not just use HashMap<String, FormattingProperty> here?

Copy link
Contributor Author

@farwyler farwyler Jun 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

That would make helix-core dependent on the lsp crate, just to get the FormattingProperty type into LanguageConfiguration.
no longer a concern with 6c29b0e


#[serde(default)]
pub diagnostic_severity: Severity,

Expand Down
22 changes: 22 additions & 0 deletions helix-view/src/document.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,11 +410,33 @@ impl Document {
let language_server = self.language_server()?;
let text = self.text.clone();
let offset_encoding = language_server.offset_encoding();

let mut properties = HashMap::new();
if let Some(fmt) = self
.language_config()
.and_then(|cfg| cfg.format.as_ref().and_then(|c| c.as_object()))
{
for (key, value) in fmt {
let prop = if let Some(s) = value.as_str() {
lsp::FormattingProperty::String(s.to_owned())
} else if let Some(b) = value.as_bool() {
lsp::FormattingProperty::Bool(b)
} else if let Some(n) = value.as_i64() {
lsp::FormattingProperty::Number(n as i32)
} else {
log::warn!("invalid formatting property type {:?} for {}", value, key);
continue;
};
Copy link
Member

Choose a reason for hiding this comment

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

I think this should work:

let prop: lsp::FormattingProperty = serde_json::from_value(value)

Copy link
Contributor Author

@farwyler farwyler Jun 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

great. i changed it to 563fccf

for (key, value) in fmt.iter() {
  if let Ok(prop) = serde_json::from_value(value.clone()) {
    properties.insert(key.to_owned(), prop);
  }
}

properties.insert(key.to_owned(), prop);
}
}

let request = language_server.text_document_formatting(
self.identifier(),
lsp::FormattingOptions {
tab_size: self.tab_width() as u32,
insert_spaces: matches!(self.indent_style, IndentStyle::Spaces(_)),
properties,
..Default::default()
},
None,
Expand Down