Skip to content

Commit

Permalink
fix: Check for a bogus root folder in workspace/didChangeWorkspaceFol…
Browse files Browse the repository at this point in the history
…ders request (#380)

Sometimes, LSP clients can send bogus folders (CWD, or homedir) as part
of added folders in didChangeWorkspaceFolders. Whenever this happens
marksman treats the root as the real root of the project folders and
recursively scans/indexes mardown documents underneath.This is
problematic when such root is a homedir because it makes marksman scan
too much and potentially fail.

Fixes #377
Potentially, addresses #373 too
  • Loading branch information
artempyanykh authored Dec 14, 2024
1 parent b9ec789 commit 8299f11
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 32 deletions.
31 changes: 31 additions & 0 deletions Marksman/Folder.fs
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,37 @@ module Folder =

let private ignoreFiles = [ ".ignore"; ".gitignore"; ".hgignore" ]

let private isRealWorkspaceFolder (root: RootPath) : bool =
let root = RootPath.toSystem root

if Directory.Exists(root) then
let markerFiles = [| ".marksman.toml" |]
let markerDirs = [| ".git"; ".hg"; ".svn" |]

let hasMarkerFile () =
Array.exists (fun marker -> File.Exists(Path.Join(root, marker))) markerFiles

let hasMarkerDir () =
Array.exists (fun marker -> Directory.Exists(Path.Join(root, marker))) markerDirs

hasMarkerDir () || hasMarkerFile ()
else
false

// Context is in
// * https://github.com/helix-editor/helix/issues/4436
// * https://github.com/artempyanykh/marksman/discussions/377
let checkWorkspaceFolderWithWarn (folderId: FolderId) : bool =
if isRealWorkspaceFolder folderId.data then
true
else
logger.warn (
Log.setMessage "Workspace folder is bogus"
>> Log.addContext "root" folderId.data
)

false

let isSingleFile folder =
match folder.data with
| SingleFile _ -> true
Expand Down
1 change: 1 addition & 0 deletions Marksman/Folder.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ module Folder =
val docs: Folder -> seq<Doc>
val docCount: Folder -> int

val checkWorkspaceFolderWithWarn: FolderId -> bool
val tryLoad: userConfig: option<Config> -> name: string -> FolderId -> option<Folder>

val singleFile: doc: Doc -> config: option<Config> -> Folder
Expand Down
33 changes: 2 additions & 31 deletions Marksman/Server.fs
Original file line number Diff line number Diff line change
Expand Up @@ -28,41 +28,12 @@ open Newtonsoft.Json.Linq
module ServerUtil =
let logger = LogProvider.getLoggerByName "ServerUtil"

let private isRealWorkspaceFolder (root: RootPath) : bool =
let root = RootPath.toSystem root

if Directory.Exists(root) then
let markerFiles = [| ".marksman.toml" |]
let markerDirs = [| ".git"; ".hg"; ".svn" |]

let hasMarkerFile () =
Array.exists (fun marker -> File.Exists(Path.Join(root, marker))) markerFiles

let hasMarkerDir () =
Array.exists (fun marker -> Directory.Exists(Path.Join(root, marker))) markerDirs

hasMarkerDir () || hasMarkerFile ()
else
false

// Remove this and related logic when https://github.com/helix-editor/helix/issues/4436 is resolved.
let checkWorkspaceFolderWithWarn (folderId: FolderId) : bool =
if isRealWorkspaceFolder folderId.data then
true
else
logger.warn (
Log.setMessage "Workspace folder is bogus"
>> Log.addContext "root" folderId.data
)

false

let extractWorkspaceFolders (par: InitializeParams) : Map<string, FolderId> =
match par.WorkspaceFolders with
| Some folders ->
folders
|> Array.map (fun { Name = name; Uri = uri } -> name, UriWith.mkRoot uri)
|> Array.filter (fun (_, folderId) -> checkWorkspaceFolderWithWarn folderId)
|> Array.filter (fun (_, folderId) -> Folder.checkWorkspaceFolderWithWarn folderId)
|> Map.ofArray
| _ ->
let rootUri =
Expand All @@ -75,7 +46,7 @@ module ServerUtil =
// No folders are configured. The client can still add folders later using a notification.
Map.empty
| Some rootUri ->
if checkWorkspaceFolderWithWarn rootUri then
if Folder.checkWorkspaceFolderWithWarn rootUri then
let rootName = RootPath.filename rootUri.data

Map.ofList [ rootName, rootUri ]
Expand Down
6 changes: 5 additions & 1 deletion Marksman/State.fs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,11 @@ module State =
for f in added do
let rootUri = UriWith.mkRoot f.Uri

let folder = Folder.tryLoad userConfig f.Name rootUri
let folder =
if Folder.checkWorkspaceFolderWithWarn rootUri then
Folder.tryLoad userConfig f.Name rootUri
else
None

match folder with
| Some folder -> yield folder
Expand Down

0 comments on commit 8299f11

Please sign in to comment.