Skip to content

Commit

Permalink
Fix some edits not being sent to LangServ (#3186)
Browse files Browse the repository at this point in the history
Some edits were not being sent by IDE to Language Server, resulting in 3003 "Invalid version" errors being returned by LangServ, and forcing full invalidation (resynchronization) of text contents in LangServ.

This change fixes such errors observed when opening a new project, creating a new project, or adding a new node to a project. 

Fixes #3094

### Important Notes

The root cause showed up to be actually two separate issues, both of them reproduced by an "opening a new project" scenario:

 1. The automatic addition of `import Standard.Visualization` line, [done internally when opening a new project in `controller::Project::initialize()`](https://github.com/enso-org/enso/blob/c14a2d81698217349394597fea778fd86bba47df/app/gui/src/controller/project.rs#L137-L141), was not reaching the Language Server. The cause of it was a race condition with [`self.model.subscribe()` in `Module::runner()`](https://github.com/enso-org/enso/blob/c14a2d81698217349394597fea778fd86bba47df/app/gui/src/model/module/synchronized.rs#L268). In particular, the addition of the import was executed before the subscription, which resulted in an edition notification being lost and not sent to LangServer. The fix employed for this is to make the `subscribe()` call synchronous during the initialization of a project, instead of scheduling it for a non-deterministic later time.
 2. There was [a bug in `synchronized::Module::edit_for_snipped()`](https://github.com/enso-org/enso/blob/7467efda592ab5d7ce29122af20020293f9ceacf/app/gui/src/model/module/synchronized.rs#L362), making it erroneously "optimize out" any code insertions detected by `TextEdit::from_prefix_postfix_differences()`. The fix employed for this was to improve the "optimizing out" condition, together with adding an accompanying test case verifying correct behavior (protecting against a future regression).

Additionally, as a drive-by improvement, some statements in `ParsedSourceFile<>::serialize()` were reordered, to make them better match how the actual contents of an .enso file are structured, and thus make it easier to read/analyze the code.
  • Loading branch information
akavel authored Dec 13, 2021
1 parent 7467efd commit 050e52b
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 21 deletions.
4 changes: 4 additions & 0 deletions app/gui/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,14 @@
- [Fixed broken node whose expression contains non-ASCII characters.][3166]
- [Fixed developer console warnings about views being created but not
registered.][3181]
- [Fixed developer console errors related to Language Server (mentioning code
3003 and "Invalid version"), occurring during project opening and after new
node cration.][3186]

[3153]: https://github.com/enso-org/enso/pull/3153
[3166]: https://github.com/enso-org/enso/pull/3166
[3181]: https://github.com/enso-org/enso/pull/3181
[3186]: https://github.com/enso-org/enso/pull/3186

# Enso 2.0.0-alpha.18 (2021-10-12)

Expand Down
5 changes: 3 additions & 2 deletions app/gui/language/parser/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,13 +183,14 @@ fn to_json_single_line(val: &impl Serialize) -> std::result::Result<String, serd
impl<M: Metadata> ParsedSourceFile<M> {
/// Serialize to the SourceFile structure,
pub fn serialize(&self) -> std::result::Result<SourceFile, serde_json::Error> {
let code = self.ast.repr();
let before_tag = "\n".repeat(NEWLINES_BEFORE_TAG);
let before_idmap = "\n";
let before_metadata = "\n";
let code = self.ast.repr();
let json_id_map = JsonIdMap::from_id_map(&self.ast.id_map(), &code);
let id_map = to_json_single_line(&json_id_map)?;
let before_metadata = "\n";
let metadata = to_json_single_line(&self.metadata)?;

let id_map_start = code.len() + before_tag.len() + METADATA_TAG.len() + before_idmap.len();
let id_map_start_bytes = Bytes::from(id_map_start);
let metadata_start = id_map_start + id_map.len() + before_metadata.len();
Expand Down
60 changes: 41 additions & 19 deletions app/gui/src/model/module/synchronized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,29 +256,32 @@ impl API for Module {
// === Synchronizing Language Server ===

impl Module {
/// The asynchronous task scheduled during struct creation which listens for all module changes
/// and send proper updates to Language Server.
async fn runner(
/// Returns the asynchronous task which listens for all module changes and sends proper updates
/// to Language Server.
fn runner(
self: Rc<Self>,
initial_ls_content: ContentSummary,
first_invalidation: impl Future<Output = FallibleResult<ParsedContentSummary>>,
) {
let first_invalidation = first_invalidation.await;
let mut ls_content = self.new_ls_content_info(initial_ls_content, first_invalidation);
) -> impl Future<Output = ()> {
let mut subscriber = self.model.subscribe();
let weak = Rc::downgrade(&self);
drop(self);

loop {
let notification = subscriber.next().await;
let this = weak.upgrade();
match (notification, this) {
(Some(notification), Some(this)) => {
debug!(this.logger, "Processing a notification: {notification:?}");
let result = this.handle_notification(&ls_content, notification).await;
ls_content = this.new_ls_content_info(ls_content.summary().clone(), result)

async move {
let first_invalidation = first_invalidation.await;
let mut ls_content = self.new_ls_content_info(initial_ls_content, first_invalidation);
let weak = Rc::downgrade(&self);
drop(self);

loop {
let notification = subscriber.next().await;
let this = weak.upgrade();
match (notification, this) {
(Some(notification), Some(this)) => {
debug!(this.logger, "Processing a notification: {notification:?}");
let result = this.handle_notification(&ls_content, notification).await;
ls_content = this.new_ls_content_info(ls_content.summary().clone(), result)
}
_ => break,
}
_ => break,
}
}
}
Expand Down Expand Up @@ -359,7 +362,7 @@ impl Module {
debug_assert_eq!(start.column, 0.column());

let edit = TextEdit::from_prefix_postfix_differences(&source, &target);
(edit.range.start != edit.range.end)
(edit.range.start != edit.range.end || !edit.text.is_empty())
.as_some_from(|| edit.move_by_lines(start.line.as_usize()))
}

Expand Down Expand Up @@ -716,4 +719,23 @@ pub mod test {
};
Runner::run(test);
}

#[test]
fn handle_insertion_edits_bug180558676() {
let source = Text::from("from Standard.Base import all\n\nmain =\n operator1 = 0.up_to 100 . to_vector . map .noise\n operator1.sort\n");
let target = Text::from("from Standard.Base import all\nimport Standard.Visualization\n\nmain =\n operator1 = 0.up_to 100 . to_vector . map .noise\n operator1.sort\n");
let edit = Module::edit_for_snipped(
&Location { line: 0.into(), column: 0.into() },
source,
target,
);
let expected = Some(TextEdit {
range: TextRange {
start: Position { line: 1, character: 0 },
end: Position { line: 1, character: 0 },
},
text: "import Standard.Visualization\n".to_string(),
});
assert_eq!(edit, expected);
}
}

0 comments on commit 050e52b

Please sign in to comment.