Skip to content

Commit

Permalink
Enter tracking for comment continuation (FuelLabs#3979)
Browse files Browse the repository at this point in the history
## Description

Closes FuelLabs#3520

![Feb-03-2023
14-20-39](https://user-images.githubusercontent.com/47993817/216722350-07bbcbb0-3f1f-4370-aa56-7dce5c22a341.gif)

- Adds two new client config options: disable/enable continuation of doc
comments (`///`) and of regular comments (`//`)
- doc comment continuation is on by default, while regular comment
continuation is off by default, since those are usually not multiline
- Supports pasting multiline text into a comment block

I implemented this using `did_change` rather than a custom LSP method or
keybindings so that this feature will be available to all LSP-enabled
editors without any additional setup.

I didn't implement anything for closing curly braces since this is
already supported in sway via the default `"editor.autoClosingBrackets":
"languageDefined"` setting in VSCode.

It will use the same indentation as the line before it, unless
auto-indentation is turned off.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.

---------

Co-authored-by: Joshua Batty <[email protected]>
  • Loading branch information
sdankel and JoshuaBatty authored Feb 7, 2023
1 parent ddd1b8b commit c170dd1
Show file tree
Hide file tree
Showing 7 changed files with 259 additions and 12 deletions.
2 changes: 2 additions & 0 deletions sway-lsp/src/capabilities/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ pub mod formatting;
pub mod highlight;
pub mod hover;
pub mod inlay_hints;
pub mod on_enter;
pub mod rename;
pub mod runnable;
pub mod semantic_tokens;

pub(crate) use code_actions::code_actions;
pub(crate) use on_enter::on_enter;
203 changes: 203 additions & 0 deletions sway-lsp/src/capabilities/on_enter.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,203 @@
use crate::{
config::OnEnterConfig,
core::{document::TextDocument, session::Session},
};
use std::sync::Arc;
use tower_lsp::{
lsp_types::{
DidChangeTextDocumentParams, DocumentChanges, OneOf,
OptionalVersionedTextDocumentIdentifier, Position, Range, TextDocumentEdit, TextEdit, Url,
WorkspaceEdit,
},
Client,
};

const NEWLINE: &str = "\n";
const COMMENT_START: &str = "//";
const DOC_COMMENT_START: &str = "///";

/// If the change was an enter keypress or pasting multiple lines in a comment, it prefixes the line(s)
/// with the appropriate comment start pattern (// or ///).
pub(crate) async fn on_enter(
config: &OnEnterConfig,
client: &Client,
session: &Arc<Session>,
temp_uri: &Url,
params: &DidChangeTextDocumentParams,
) {
if !(params.content_changes[0].text.contains(NEWLINE)) {
return;
}

let mut workspace_edit = None;
let text_document = session
.get_text_document(temp_uri)
.expect("could not get text document");

if config.continue_doc_comments.unwrap_or(false) {
workspace_edit = get_comment_workspace_edit(DOC_COMMENT_START, params, &text_document);
}

if config.continue_comments.unwrap_or(false) && workspace_edit.is_none() {
workspace_edit = get_comment_workspace_edit(COMMENT_START, params, &text_document);
}

// Apply any edits.
if let Some(edit) = workspace_edit {
if let Err(err) = client.apply_edit(edit).await {
tracing::error!("on_enter failed to apply edit: {}", err);
}
}
}

fn get_comment_workspace_edit(
start_pattern: &str,
change_params: &DidChangeTextDocumentParams,
text_document: &TextDocument,
) -> Option<WorkspaceEdit> {
let range = change_params.content_changes[0]
.range
.expect("change is missing range");
let line = text_document.get_line(range.start.line as usize);
if line.trim().starts_with(start_pattern) {
let uri = change_params.text_document.uri.clone();
let text = change_params.content_changes[0].text.clone();

let indentation = &line[..line.find(start_pattern).unwrap_or(0)];
let mut edits = vec![];

// To support pasting multiple lines in a comment, we need to add the comment start pattern after each newline,
// except the last one.
let lines: Vec<_> = text.split(NEWLINE).collect();
lines.iter().enumerate().for_each(|(i, _)| {
if i < lines.len() - 1 {
let position =
Position::new(range.start.line + (i as u32) + 1, indentation.len() as u32);
edits.push(OneOf::Left(TextEdit {
new_text: format!("{start_pattern} "),
range: Range::new(position, position),
}));
}
});
let edit = TextDocumentEdit {
text_document: OptionalVersionedTextDocumentIdentifier {
// Use the original uri to make updates, not the temporary one from the session.
uri,
version: None,
},
edits,
};
Some(WorkspaceEdit {
document_changes: Some(DocumentChanges::Edits(vec![edit])),
..Default::default()
})
} else {
None
}
}

#[cfg(test)]
mod tests {
use super::*;
use crate::utils::test::get_absolute_path;
use tower_lsp::lsp_types::{
AnnotatedTextEdit, TextDocumentContentChangeEvent, VersionedTextDocumentIdentifier,
};

fn assert_text_edit(
actual: &OneOf<TextEdit, AnnotatedTextEdit>,
new_text: String,
line: u32,
character: u32,
) {
match actual {
OneOf::Left(edit) => {
let position = Position { line, character };
let expected = TextEdit {
new_text,
range: Range {
start: position,
end: position,
},
};
assert_eq!(*edit, expected);
}
OneOf::Right(_) => panic!("expected left"),
}
}

#[test]
fn get_comment_workspace_edit_double_slash_indented() {
let path = get_absolute_path("sway-lsp/test/fixtures/diagnostics/dead_code/src/main.sw");
let uri = Url::from_file_path(path.clone()).unwrap();
let text_document =
TextDocument::build_from_path(path.as_str()).expect("failed to build document");
let params = DidChangeTextDocumentParams {
text_document: VersionedTextDocumentIdentifier { uri, version: 1 },
content_changes: vec![TextDocumentContentChangeEvent {
range: Some(Range {
start: Position {
line: 47,
character: 34,
},
end: Position {
line: 47,
character: 34,
},
}),
range_length: Some(0),
text: "\n ".to_string(),
}],
};

let result = get_comment_workspace_edit(COMMENT_START, &params, &text_document)
.expect("workspace edit");
let changes = result.document_changes.expect("document changes");
let edits = match changes {
DocumentChanges::Edits(edits) => edits,
DocumentChanges::Operations(_) => panic!("expected edits"),
};

assert_eq!(edits.len(), 1);
assert_eq!(edits[0].edits.len(), 1);
assert_text_edit(&edits[0].edits[0], "// ".to_string(), 48, 4);
}

#[test]
fn get_comment_workspace_edit_triple_slash_paste() {
let path = get_absolute_path("sway-lsp/test/fixtures/diagnostics/dead_code/src/main.sw");
let uri = Url::from_file_path(path.clone()).unwrap();
let text_document =
TextDocument::build_from_path(path.as_str()).expect("failed to build document");
let params = DidChangeTextDocumentParams {
text_document: VersionedTextDocumentIdentifier { uri, version: 1 },
content_changes: vec![TextDocumentContentChangeEvent {
range: Some(Range {
start: Position {
line: 41,
character: 4,
},
end: Position {
line: 41,
character: 34,
},
}),
range_length: Some(30),
text: "fn not_used2(input: u64) -> u64 {\n return input + 1;\n}".to_string(),
}],
};

let result = get_comment_workspace_edit(DOC_COMMENT_START, &params, &text_document)
.expect("workspace edit");
let changes = result.document_changes.expect("document changes");
let edits = match changes {
DocumentChanges::Edits(edits) => edits,
DocumentChanges::Operations(_) => panic!("expected edits"),
};

assert_eq!(edits.len(), 1);
assert_eq!(edits[0].edits.len(), 2);
assert_text_edit(&edits[0].edits[0], "/// ".to_string(), 42, 0);
assert_text_edit(&edits[0].edits[1], "/// ".to_string(), 43, 0);
}
}
37 changes: 29 additions & 8 deletions sway-lsp/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,31 @@ pub struct Config {
pub inlay_hints: InlayHintsConfig,
#[serde(default)]
pub diagnostic: DiagnosticConfig,
#[serde(default)]
pub on_enter: OnEnterConfig,
#[serde(default, skip_serializing)]
trace: TraceConfig,
}

#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Default)]
struct TraceConfig {}

// Options for debugging various parts of the server
// Options for debugging various parts of the server.
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct DebugConfig {
pub show_collected_tokens_as_warnings: Warnings,
}

// Options for displaying compiler diagnostics
impl Default for DebugConfig {
fn default() -> Self {
Self {
show_collected_tokens_as_warnings: Warnings::Default,
}
}
}

// Options for displaying compiler diagnostics.
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct DiagnosticConfig {
Expand All @@ -43,6 +53,7 @@ impl Default for DiagnosticConfig {
}
}

// Options for confguring server logging.
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
pub struct LoggingConfig {
#[serde(with = "LevelFilterDef")]
Expand Down Expand Up @@ -80,6 +91,7 @@ pub enum Warnings {
Typed,
}

// Options for configuring inlay hints.
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct InlayHintsConfig {
Expand All @@ -91,20 +103,29 @@ pub struct InlayHintsConfig {
pub max_length: Option<usize>,
}

impl Default for DebugConfig {
impl Default for InlayHintsConfig {
fn default() -> Self {
Self {
show_collected_tokens_as_warnings: Warnings::Default,
render_colons: true,
type_hints: true,
max_length: Some(25),
}
}
}

impl Default for InlayHintsConfig {
// Options for additional behavior when the user presses enter.
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct OnEnterConfig {
pub continue_doc_comments: Option<bool>,
pub continue_comments: Option<bool>,
}

impl Default for OnEnterConfig {
fn default() -> Self {
Self {
render_colons: true,
type_hints: true,
max_length: Some(25),
continue_doc_comments: Some(true),
continue_comments: Some(false),
}
}
}
Expand Down
6 changes: 5 additions & 1 deletion sway-lsp/src/core/document.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use tower_lsp::lsp_types::{Position, Range, TextDocumentContentChangeEvent};

use crate::error::DocumentError;

#[derive(Debug)]
#[derive(Debug, Clone)]
pub struct TextDocument {
#[allow(dead_code)]
language_id: String,
Expand All @@ -30,6 +30,10 @@ impl TextDocument {
&self.uri
}

pub fn get_line(&self, line: usize) -> String {
self.content.line(line).to_string()
}

pub fn apply_change(&mut self, change: &TextDocumentContentChangeEvent) {
let edit = self.build_edit(change);

Expand Down
11 changes: 11 additions & 0 deletions sway-lsp/src/core/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,17 @@ impl Session {
Ok(())
}

/// Get the document at the given [Url].
pub fn get_text_document(&self, url: &Url) -> Result<TextDocument, DocumentError> {
self.documents
.try_get(url.path())
.try_unwrap()
.ok_or_else(|| DocumentError::DocumentNotFound {
path: url.path().to_string(),
})
.map(|document| document.clone())
}

/// Update the document at the given [Url] with the Vec of changes returned by the client.
pub fn update_text_document(
&self,
Expand Down
9 changes: 7 additions & 2 deletions sway-lsp/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,12 +248,17 @@ impl LanguageServer for Backend {
}

async fn did_change(&self, params: DidChangeTextDocumentParams) {
let config = self.config.read().on_enter.clone();
match self.get_uri_and_session(&params.text_document.uri) {
Ok((uri, session)) => {
// handle on_enter capabilities if they are enabled
capabilities::on_enter(&config, &self.client, &session, &uri.clone(), &params)
.await;

// update this file with the new changes and write to disk
match session.write_changes_to_file(&uri, params.content_changes) {
Ok(_) => {
self.parse_project(uri, params.text_document.uri, session.clone())
self.parse_project(uri, params.text_document.uri.clone(), session.clone())
.await;
}
Err(err) => tracing::error!("{}", err.to_string()),
Expand Down Expand Up @@ -281,7 +286,7 @@ impl LanguageServer for Backend {
sync::edit_manifest_dependency_paths(&manifest, temp_manifest_path)
}
});
self.parse_project(uri, params.text_document.uri, session.clone())
self.parse_project(uri, params.text_document.uri, session)
.await;
}
Err(err) => tracing::error!("{}", err.to_string()),
Expand Down
3 changes: 2 additions & 1 deletion sway-lsp/test/fixtures/diagnostics/dead_code/src/main.sw
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,12 @@ fn not_used1() -> bool {
}
return true;
}
/// Comments about unused code
/// Doc comments about unused code
fn not_used2(input: u64) -> u64 {
return input + 1;
}

fn alive() -> NotUsedEnumVariant {
// Comments about return value
return NotUsedEnumVariant::A;
}

0 comments on commit c170dd1

Please sign in to comment.