Skip to content

Add support for file and folder operations (create, rename, move) to workspace edits #267

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

Merged
merged 12 commits into from
Dec 13, 2020

Conversation

banacorn
Copy link
Contributor

Instead of having TextDocumentEdit in WorkspaceEdit

data WorkspaceEdit =
  WorkspaceEdit
    { _changes         :: Maybe WorkspaceEditMap
    , _documentChanges :: Maybe (List TextDocumentEdit)
    } deriving (Show, Read, Eq)

It is now replaced by a new type called DocumentChange

data WorkspaceEdit =
  WorkspaceEdit
    { _changes         :: Maybe WorkspaceEditMap
    , _documentChanges :: Maybe (List DocumentChange)
    } deriving (Show, Read, Eq)

Which is just a synonym of union of WorkspaceEdit and other operations

type DocumentChange = TextDocumentEdit |? CreateFile |? RenameFile |? DeleteFile

So that it complies with the spec as suggested by @EggBaconAndSpam in #134 (comment)


Here's how to construct a DocumentChange that creates a file for example:

InR (InL (CreateFile FileResourceChangeCreate "/path/to/new/file" Nothing))

But it's a bit wordy and unsafe because of the FileResoureChangeKind.
I'm considering hiding these constructors and export only smart constructors.

@jneira jneira requested a review from lukel97 December 10, 2020 06:26
@banacorn
Copy link
Contributor Author

@bubba I should make a PR on lsp-test, right?
In that case, lsp-test would have to depend on this commit of lsp then 👀

@lukel97
Copy link
Collaborator

lukel97 commented Dec 10, 2020

Hi @banacorn, thanks for this PR! Looks like there's a bit of a chicken and the egg situation with lap-types and lsp-test. You can create a branch on lsp-test for now and reference that commit in cabal.project, and then once this is merged we can merge the PRs for lsp-test

Copy link
Collaborator

@lukel97 lukel97 left a comment

Choose a reason for hiding this comment

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

Thanks, this is great! Just some minor nitpicks about the string literal tag. Out of curiosity, are you using this for your own language server?

@banacorn
Copy link
Contributor Author

Yes, I'm building a language server for my toy language.
I have a button that exports all proof obligations to a new file, that's why I need this feature.

I'm planning on building a language server for Agda next.

@lukel97
Copy link
Collaborator

lukel97 commented Dec 11, 2020

Whoops, looks like fix keyword in the PR message closes PRs across GitHub accounts

Copy link
Collaborator

@lukel97 lukel97 left a comment

Choose a reason for hiding this comment

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

LGTM now. Let me know when you're happy/referencing the new lsp-test commit and I'll merge

@lukel97
Copy link
Collaborator

lukel97 commented Dec 12, 2020

Looks like its passing now. Want me to merge @banacorn ?

@banacorn
Copy link
Contributor Author

Yes, please! Thank you!

@lukel97 lukel97 merged commit 36b655d into haskell:master Dec 13, 2020
lukel97 added a commit that referenced this pull request Feb 27, 2021
lukel97 added a commit that referenced this pull request Feb 27, 2021
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.

2 participants