Rewrite to use `remark-language-server` #65

remcohaszing · 2021-12-15T13:22:41Z

Initial checklist

I read the support docs
I read the contributing guide
I agree to follow the code of conduct
I searched issues and couldn’t find anything (or linked relevant results below)
If applicable, I’ve added docs and tests

Description of changes

This migrates the extension to use remark-language-server which is introduced
in remarkjs/remark#924.

This basically reimplements the extension from scratch andrepurposes it.

Old behavior:

Format code using a custom command.

New behavior:

Use the same configuration as remark-cli
Register an actual formatter
Support ESM configurations
Support diagnostics
Support quick fixes

Closes #17
Closes #41
Closes #58
Closes #61

ChristianMurphy

Thanks @remcohaszing!
It looks like CI is getting stuck at

npm run clean && npm run lint && npm run compile

npm ERR! Missing script: "clean"

src/extension.js

package.json

wooorm · 2021-12-16T18:28:34Z

👍 after build succeeds!

This migrates the extension to use remark-language-server which is introduced in remarkjs/remark#924. This basically reimplements the extension from scratch andrepurposes it. Old behavior: - Format code using a custom command. New behavior: - Use the same configuration as remark-cli - Register an actual formatter - Support ESM configurations - Support diagnostics

This uses esbuild to bundle and minify the extension.

It has also been renamed to lower case to be consistent with the rest of the unified ecosystem.

remcohaszing · 2021-12-27T19:04:37Z

test/index.test.js

+        'Marker style should be `*`',
+        'Marker style should be `*`',
+        'Marker style should be `*`'
+      ])


This is more about testing the language server can be loaded than about testing its functionality, as that’s tested upstream in unified-language-server.

remcohaszing · 2021-12-27T19:07:04Z

package.json

  "scripts": {
-    "clean": "rimraf out",
+    "build:language-server": "esbuild node_modules/remark-language-server/index.js --bundle --platform=node --minify --format=cjs --outfile=out/remark-language-server.js",


ESBuild can’t process require statements which load NodeJS builtins, so the language server is built as a CJS module. It is able to use dynamic import() this way, so everything works fine this way (this is also tested).

remcohaszing · 2021-12-27T19:08:17Z

test/index.test.js

+/**
+ * @returns {Promise<void>}
+ */
+module.exports.run = () =>


The test file must be loadable by @vscode/test-electron, meaning it has to use CJS.

wooorm

Only really prose nits!

.vscode/launch.json

src/extension.js

readme.md

VSCode Extensions need to use CJS, but this is built using esbuild anyway.

wooorm

wip review

package.json

readme.md

Co-authored-by: Titus <[email protected]>

This adds support for quick fixes.

wooorm · 2022-01-13T09:56:32Z

@remcohaszing I see you sometimes pushing updates to dependencies. Are you treating this PR is ready to go? Because I’m waiting for the tests to turn green and I was under the impression that there are still some things being worked on?

remcohaszing · 2022-01-13T11:26:03Z

I was hoping that might automagically fix it, but it didn’t, not that I really expected that.

Previously the Windows build failed, but now even the Linux build fails. It used to work on Linux.

Manual testing shows the following error.

Debugger listening on ws://127.0.0.1:6009/f22fc09c-2ff5-4353-ab6a-fab913fbf799
For help, see: https://nodejs.org/en/docs/inspector
[Error - 12:08:42 PM] Connection to server is erroring. Shutting down server.
/home/remco/Projects/vscode-remark/out/remark-language-server.js:25
${JSON.stringify(w.error,void 0,4)}`):i.error("Received response message without id. No further error information provided.");else{let E=String(w.id),h=x[E];if(Os(w,h),h){delete x[E];try{if(w.error){let b=w.error;h.reject(new z.ResponseError(b.code,b.message,b.data))}else if(w.result!==void 0)h.resolve(w.result);else throw new Error("Should never happen.")}catch(b){b.message?i.error(`Response handler '${h.method}' failed with message: ${b.message}`):i.error(`Response handler '${h.method}' failed unexpectedly.`)}}}}function ui(w){if(Qe())return;let E,h;if(w.method===co.type.method)h=b=>{let _=b.id,D=v[String(_)];D&&D.cancel()};else{let b=p[w.method];b&&(h=b.handler,E=b.type)}if(h||f)try{Ts(w),h?w.params===void 0?(E!==void 0&&E.numberOfParams!==0&&E.parameterStructures!==z.ParameterStructures.byName&&i.error(`Notification ${w.method} defines ${E.numberOfParams} params but recevied none.`),h()):Array.isArray(w.params)?(E!==void 0&&(E.parameterStructures===z.ParameterStructures.byName&&i.error(`Notification ${w.method} defines parameters by name but received parameters by position`),E.numberOfParams!==w.params.length&&i.error(`Notification ${w.method} defines ${E.numberOfParams} params but received ${w.params.length} argumennts`)),h(...w.params)):(E!==void 0&&E.parameterStructures===z.ParameterStructures.byPosition&&i.error(`Notification ${w.method} defines parameters by position but received parameters by name`),h(w.params)):f&&f(w.method,w.params)}catch(b){b.message?i.error(`Notification handler '${w.method}' failed with message: ${b.message}`):i.error(`Notification handler '${w.method}' failed unexpectedly.`)}else ne.fire(w)}function Rs(w){if(!w){i.error("Received empty message.");return}i.error(`Received message which is neither a response nor a notification message:
                                                                                                                                                                                                                              ^

Zu [Error]: Unhandled method client/registerCapability
    at Fs (/home/remco/Projects/vscode-remark/out/remark-language-server.js:25:223)
    at Cs (/home/remco/Projects/vscode-remark/out/remark-language-server.js:24:11203)
    at Immediate.<anonymous> (/home/remco/Projects/vscode-remark/out/remark-language-server.js:24:11056)
    at processImmediate (node:internal/timers:464:21) {
  code: -32601,
  data: undefined
}

Unfortunately I also couldn’t easily get source maps to work for debugging. Changing line 11 in src/extension.js to const args = [require.resolve('remark-language-server'), '--stdio'] helps with debugging.

It would also help if someone can manually confirm it works on macOS or Windows. Especially Windows development setups are pretty alien to me. Can people just invoke node?

I also tried using LSP over node-ipc instead of stdio earlier. This caused issues. I think it disallows to use ESM.

I currently have limited time to look into this, but I do really want to finish this!

wooorm · 2022-01-27T08:42:21Z

ctx.asAbsolutePath seems to be in the right direction: https://github.com/angular/vscode-ng-language-service/blob/2deee1829a386f2f0981116378508b71ae974f3a/client/src/client.ts#L499-L514.

so ubuntu seems to work always, macOS is flakey, and windows fails always?

remcohaszing · 2022-01-27T08:59:49Z

Kind of but not quite? I see an Ubuntu build failing while typing this comment.

I think using the ipc transport might also make it a bit more stable.

This test doesn’t rely on any timeouts.

wooorm · 2022-07-12T19:31:06Z

Can you mention me again when this is in need of a review?

wooorm · 2022-07-13T20:14:21Z

We probably should also remove the lockfile, and clean other repo things, in further PRs, btw!

remcohaszing · 2022-07-13T20:29:10Z

Omg all checks have passed! 🎉

I agree we can do some more cleanups in other PRs. Let’s focus on this one first though. I’ll fix various quirks after this PR has been merged.

wooorm · 2022-07-13T20:45:36Z

Sweet, landed!
Let me know what your plans are, to first update some more things (?), and to release something?

remcohaszing mentioned this pull request Dec 15, 2021

Rename vscode-remark to remark #66

Closed

4 tasks

ChristianMurphy requested review from asbjornu, mrmlnc and a team December 15, 2021 14:01

ChristianMurphy reviewed Dec 15, 2021

View reviewed changes

src/extension.js Show resolved Hide resolved

package.json Show resolved Hide resolved

remcohaszing marked this pull request as draft December 24, 2021 13:18

remcohaszing marked this pull request as ready for review December 27, 2021 17:47

remcohaszing added 8 commits December 27, 2021 19:56

Remove console option from launch.json

024e7df

Build extension

28c22b9

This uses esbuild to bundle and minify the extension.

Fix linting issues

1ce06c0

Change path to remark-language-server

aa29205

Update ignore files

6b477e7

Rewrite readme

ff50dce

It has also been renamed to lower case to be consistent with the rest of the unified ecosystem.

Restore remarkConfig in original location

912c630

remcohaszing commented Dec 27, 2021

View reviewed changes

wooorm reviewed Dec 27, 2021

View reviewed changes

wooorm changed the title ~~Use remark-language-server~~ Use remark-language-server Dec 27, 2021

remcohaszing added 7 commits December 27, 2021 20:27

Use ESM for src

69a3309

VSCode Extensions need to use CJS, but this is built using esbuild anyway.

Use code tags to indicate VSCode controls

926f5aa

Apply some small documentation fixes

6c47159

Add spacing between <kbd> tags

93b5db0

Add What is this? section.

e9d80f9

Fix typo

83ceeab

Update usage example

5d99ed7

remcohaszing requested a review from ChristianMurphy December 28, 2021 10:41

Make remark lower case everywhere

bb8a697

wooorm reviewed Dec 28, 2021

View reviewed changes

package.json Outdated Show resolved Hide resolved

readme.md Outdated Show resolved Hide resolved

readme.md Outdated Show resolved Hide resolved

readme.md Outdated Show resolved Hide resolved

Use extension to indicate VSCode extension

1222192

Co-authored-by: Titus <[email protected]>

remcohaszing added 2 commits January 3, 2022 14:02

Update dependencies

2f1cb99

This adds support for quick fixes.

Update dependencies

efc4aad

remcohaszing mentioned this pull request Jan 24, 2022

Fix workspace initialization unifiedjs/unified-language-server#27

Merged

5 tasks

remcohaszing added 8 commits January 25, 2022 09:52

Update dependencies

affd708

Increase sleep in tests

1347a90

Increase timeout in test

dbc623a

Remove undefined heading from toc

be8328f

Double sleep to see if it helpsfor windows

bd0aecb

Switch to node-ipc protocol

daf9a0d

Increase sleep in test

163216a

Use context.asAbsolutePath to resolve extension

341439d

remcohaszing mentioned this pull request Jan 30, 2022

The language server cwd is not a good fallback for unified-engine cwd unifiedjs/unified-language-server#29

Closed

4 tasks

remcohaszing added 4 commits July 11, 2022 21:27

Update dependencies

0974aa1

Use more stable test

f4c1f02

This test doesn’t rely on any timeouts.

Change language client name and id to remark

53e597f

Fix Windows path issue

89ea2c2

remcohaszing mentioned this pull request Jul 12, 2022

Resolving package.json should rely on the vfile cwd, not process remarkjs/remark-github#39

Closed

4 tasks

Update lockfile

4f83743

ChristianMurphy approved these changes Jul 13, 2022

View reviewed changes

wooorm approved these changes Jul 13, 2022

View reviewed changes

wooorm changed the title ~~Use remark-language-server~~ Rewrite to use remark-language-server Jul 13, 2022

wooorm merged commit da00a11 into remarkjs:main Jul 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite to use `remark-language-server` #65

Rewrite to use `remark-language-server` #65

remcohaszing commented Dec 15, 2021 •

edited

Loading

ChristianMurphy left a comment

wooorm commented Dec 16, 2021

remcohaszing Dec 27, 2021

remcohaszing Dec 27, 2021

remcohaszing Dec 27, 2021

wooorm left a comment

wooorm left a comment

wooorm commented Jan 13, 2022

remcohaszing commented Jan 13, 2022

wooorm commented Jan 27, 2022 •

edited

Loading

remcohaszing commented Jan 27, 2022

wooorm commented Jul 12, 2022

wooorm commented Jul 13, 2022

remcohaszing commented Jul 13, 2022

wooorm commented Jul 13, 2022

Rewrite to use remark-language-server #65

Rewrite to use remark-language-server #65

Conversation

remcohaszing commented Dec 15, 2021 • edited Loading

Initial checklist

Description of changes

ChristianMurphy left a comment

Choose a reason for hiding this comment

wooorm commented Dec 16, 2021

remcohaszing Dec 27, 2021

Choose a reason for hiding this comment

remcohaszing Dec 27, 2021

Choose a reason for hiding this comment

remcohaszing Dec 27, 2021

Choose a reason for hiding this comment

wooorm left a comment

Choose a reason for hiding this comment

wooorm left a comment

Choose a reason for hiding this comment

wooorm commented Jan 13, 2022

remcohaszing commented Jan 13, 2022

wooorm commented Jan 27, 2022 • edited Loading

remcohaszing commented Jan 27, 2022

wooorm commented Jul 12, 2022

wooorm commented Jul 13, 2022

remcohaszing commented Jul 13, 2022

wooorm commented Jul 13, 2022

Rewrite to use `remark-language-server` #65

Rewrite to use `remark-language-server` #65

remcohaszing commented Dec 15, 2021 •

edited

Loading

wooorm commented Jan 27, 2022 •

edited

Loading