Skip to content

Conversation

@GideonKoenig
Copy link
Contributor

Related to #1243.

Summary of Changes

Adds a language server service that provides the necessary components for the graphical editor.

@GideonKoenig
Copy link
Contributor Author

Currently, this PR is still missing the necessary tests for the parser components.
Tests for the Document Parser will get added.

Testing components that are not related to ParseDocument (specifically, OpenSyncChannel, CloseSyncChannel, GetBuildings, and GetDocumentation) is currently not planned.

@lars-reimann If these components require testing, please let me know.

@github-actions
Copy link

github-actions bot commented Jan 21, 2025

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ REPOSITORY git_diff yes no 0.15s
✅ TYPESCRIPT eslint 25 0 0 4.72s
✅ TYPESCRIPT prettier 25 0 0 1.2s

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by OX Security

@codecov
Copy link

codecov bot commented Jan 21, 2025

Codecov Report

Attention: Patch coverage is 70.43796% with 243 lines in your changes missing coverage. Please review.

Project coverage is 97.99%. Comparing base (fdbefe3) to head (69b8e71).

Files with missing lines Patch % Lines
...uage/graphical-editor/graphical-editor-provider.ts 30.66% 104 Missing ⚠️
...e/graphical-editor/ast-parser/tools/debug-utils.ts 9.43% 48 Missing ⚠️
.../language/graphical-editor/ast-parser/statement.ts 62.33% 29 Missing ⚠️
...g/src/language/graphical-editor/ast-parser/call.ts 84.02% 27 Missing ⚠️
...src/language/graphical-editor/ast-parser/parser.ts 81.25% 21 Missing ⚠️
...language/graphical-editor/ast-parser/expression.ts 86.53% 7 Missing ⚠️
...afe-ds-lang/src/language/graphical-editor/types.ts 83.33% 4 Missing ⚠️
...g/src/language/graphical-editor/ast-parser/edge.ts 95.00% 2 Missing ⚠️
...fe-ds-lang/src/language/graphical-editor/global.ts 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1315      +/-   ##
==========================================
- Coverage   99.73%   97.99%   -1.74%     
==========================================
  Files         120      135      +15     
  Lines       13031    13853     +822     
  Branches     4266     4438     +172     
==========================================
+ Hits        12996    13575     +579     
- Misses         35      277     +242     
- Partials        0        1       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@GideonKoenig
Copy link
Contributor Author

GideonKoenig commented Jan 21, 2025

Edit: Has been resolved.

@lars-reimann
Before reviewing this, see my comment here.

@GideonKoenig
Copy link
Contributor Author

GideonKoenig commented Jan 21, 2025

Bug 1
Has been resolved, among some other small issues.

Bug 2
This seems the be an issue with langium, or an issue with how I use the provided services.

this.logger.error(`Parsing document <${uri.path}>`);
const document = await this.LangiumDocuments.getOrCreateDocument(uri);
this.logger.error(`Document created <${document.uri.path}>`);
await this.DocumentBuilder.build([document]);
this.logger.error(`Document built <${document.uri.path}>`);

I added these debug logging statements in the parseDocument function in the language server, then opened first the complex-titanic.sds file in the editor, then closed the graphical editor, and the opened .sds document, then opened the simple-titanic.sds file in the text editor, and then the graphical editor.

These log statements were produced after opening the simple-titanic.sds file:

2025-01-22 00:16:33.692 [error] [Graphical Editor] Parsing document </c:/Users/Gideon/projects/safe-ds/packages/safe-ds-editor/samples/simple-titanic.sds>
2025-01-22 00:16:33.693 [error] [Graphical Editor] Document created </c:/Users/Gideon/projects/safe-ds/packages/safe-ds-editor/samples/complex-titanic.sds>
2025-01-22 00:16:33.693 [error] [Graphical Editor] Document built </c:/Users/Gideon/projects/safe-ds/packages/safe-ds-editor/samples/complex-titanic.sds>

So it seems that this line:

const document = await this.LangiumDocuments.getOrCreateDocument(uri);

produces the old document, when using the new uri.

Edit: This has been resolved.
The root issue was, that even though the langium methods accept vscode URI objects, they call uri.toString() which resolved to the proper uri path for langium uri object, but to <Object, Object> for vscode URI objects. Because of this, their internal document mapping breaks, and it always returns the first entry in the map.

@GideonKoenig
Copy link
Contributor Author

@lars-reimann I have added test coverage for the most important code paths.
Debugging methods, and the service registration of SafeDsGraphicalEditorProvider have not been tested.
Is there anything else missing for this PR?

@lars-reimann
Copy link
Member

No, this should be alright now. It will still be a while until I merge your PRs, but that need not bother you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants