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.

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

Add likely project root as fallback cwd #33

Merged
merged 9 commits into from
Mar 4, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
85 changes: 59 additions & 26 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@
*/

import path from 'node:path'
import process from 'node:process'
import {PassThrough} from 'node:stream'
import {URL, pathToFileURL, fileURLToPath} from 'node:url'

import {findUp, pathExists} from 'find-up'
import {loadPlugin} from 'load-plugin'
import {engine} from 'unified-engine'
import {VFile} from 'vfile'
Expand Down Expand Up @@ -139,10 +139,15 @@ function vfileMessageToDiagnostic(message) {
* Convert language server protocol text document to a vfile.
*
* @param {TextDocument} document
* @param {string} cwd
* @returns {VFile}
*/
function lspDocumentToVfile(document) {
return new VFile({path: new URL(document.uri), value: document.getText()})
function lspDocumentToVfile(document, cwd) {
return new VFile({
cwd,
path: new URL(document.uri),
value: document.getText()
})
}

/**
Expand Down Expand Up @@ -179,32 +184,60 @@ export function configureUnifiedLanguageServer(
async function processDocuments(textDocuments, alwaysStringify = false) {
// LSP uses `file:` URLs (hrefs), `unified-engine` expects a paths.
// `process.cwd()` does not add a final slash, but `file:` URLs often do.
const workspacesAsPaths = [...workspaces].map((d) =>
fileURLToPath(d.replace(/\/$/, ''))
)
const workspacesAsPaths = [...workspaces]
.map((d) => d.replace(/[/\\]?$/, ''))
// Sort the longest (closest to the file) first.
.sort((a, b) => b.length - a.length)
/** @type {Map<string, Array<VFile>>} */
const workspacePathToFiles = new Map()

if (workspacesAsPaths.length === 0) {
workspacesAsPaths.push(process.cwd())
}
await Promise.all(
textDocuments.map(async (textDocument) => {
/** @type {string | undefined} */
let cwd
if (workspaces.size === 0) {
cwd = await findUp(
async (dir) => {
const pkgExists = await pathExists(path.join(dir, 'package.json'))
if (pkgExists) {
return dir
}

for (const textDocument of textDocuments) {
const file = lspDocumentToVfile(textDocument)
const [cwd] = workspacesAsPaths
// Every workspace that includes the document.
.filter((d) => file.path.slice(0, d.length + 1) === d + path.sep)
// Sort the longest (closest to the file) first.
.sort((a, b) => b.length - a.length)

// This presumably should not occur: a file outside a workspace.
// So ignore the file.
/* c8 ignore next */
if (!cwd) continue

const files = workspacePathToFiles.get(cwd) || []
workspacePathToFiles.set(cwd, [...files, file])
}
const gitExists = await pathExists(path.join(dir, '.git'))
// Git doesn’t allow us to commit files in a .git folder.
wooorm marked this conversation as resolved.
Show resolved Hide resolved
/* c8 ignore start */
if (gitExists) {
return dir
}
/* c8 ignore stop */
wooorm marked this conversation as resolved.
Show resolved Hide resolved
},
{
cwd: path.dirname(fileURLToPath(textDocument.uri)),
type: 'directory'
}
)
} else {
// Because the workspaces are sorted longest to shortest, the first
// match is closest to the file.
const ancestor = workspacesAsPaths.find((d) =>
textDocument.uri.startsWith(d + '/')
)
if (ancestor) {
cwd = fileURLToPath(ancestor)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I recommend switching between the 2 behaviors based on workspacesAsPaths.length > 0, instead of using one as a fallback for both o workspaces existing and a file not in a workspace.

Seems easier to reason about and debug


// This presumably should not occur: a file outside a workspace.
// So ignore the file.
/* c8 ignore next */
if (!cwd) return

const file = lspDocumentToVfile(textDocument, cwd)

const files = workspacePathToFiles.get(cwd) || []
workspacePathToFiles.set(cwd, [...files, file])
})
)

/** @type {Array<Promise<Array<VFile>>>} */
const promises = []
Expand Down Expand Up @@ -308,7 +341,7 @@ export function configureUnifiedLanguageServer(

for (const file of files) {
// VFile uses a file path, but LSP expects a file URL as a string.
const uri = String(pathToFileURL(file.path))
const uri = String(pathToFileURL(path.resolve(file.cwd, file.path)))
connection.sendDiagnostics({
uri,
version: documentVersions.get(uri),
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
],
"dependencies": {
"@types/unist": "^2.0.0",
"find-up": "^6.0.0",
"load-plugin": "^4.0.0",
"unified-engine": "^9.0.0",
"vfile": "^5.0.0",
Expand Down
6 changes: 3 additions & 3 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import * as exports from 'unified-language-server'

const sleep = promisify(setTimeout)

const delay = process.platform === 'win32' ? 800 : 400
const delay = process.platform === 'win32' ? 1000 : 400
const timeout = 10_000

test('exports', (t) => {
Expand Down Expand Up @@ -890,8 +890,8 @@ test('`textDocument/codeAction` (and diagnostics)', async (t) => {

test('`initialize` w/ nothing', async (t) => {
const stdin = new PassThrough()
const cwd = new URL('.', import.meta.url)
const promise = execa('node', ['remark-with-cwd.js', '--stdio'], {
const cwd = new URL('..', import.meta.url)
const promise = execa('node', ['./test/remark-with-cwd.js', '--stdio'], {
cwd: fileURLToPath(cwd),
input: stdin,
timeout
Expand Down
3 changes: 2 additions & 1 deletion test/remark-with-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ import {createUnifiedLanguageServer} from '../index.js'
createUnifiedLanguageServer({
processorName: 'remark',
processorSpecifier: 'remark',
plugins: ['./one-error.js']
// This is resolved from the directory containing package.json
plugins: ['./test/one-error.js']
})
3 changes: 2 additions & 1 deletion test/remark-with-warnings.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ import {createUnifiedLanguageServer} from '../index.js'
createUnifiedLanguageServer({
processorName: 'remark',
processorSpecifier: 'remark',
plugins: ['./lots-of-warnings.js']
// This is resolved from the directory containing package.json
plugins: ['./test/lots-of-warnings.js']
})