Skip to content

implement textDocument/willSaveWaitUntil#46

Open
dchinmay2 wants to merge 2 commits into
zbelial:masterfrom
dchinmay2:willSaveWaitUntil
Open

implement textDocument/willSaveWaitUntil#46
dchinmay2 wants to merge 2 commits into
zbelial:masterfrom
dchinmay2:willSaveWaitUntil

Conversation

@dchinmay2

Copy link
Copy Markdown

No description provided.

@dchinmay2

Copy link
Copy Markdown
Author

Same motivation as before: zls autofixes

@dchinmay2

Copy link
Copy Markdown
Author

@dchinmay2 dchinmay2 marked this pull request as draft February 22, 2025 10:26
@dchinmay2 dchinmay2 marked this pull request as ready for review February 22, 2025 13:19
@zbelial

zbelial commented Mar 27, 2025

Copy link
Copy Markdown
Owner

Hi @p00f , sorry for this sooooo late response, I was too busy recently, and will be busy for months too, so may be it'll take a long time before I responding next time , sorry in advance.
And thanks very much for your contribution.

I have read your PR and have some questions, I'd added some review comments.

Thanks again.

@dchinmay2

Copy link
Copy Markdown
Author

Did you forget to hit send on the review?

@zbelial

zbelial commented Mar 27, 2025

Copy link
Copy Markdown
Owner

I don't think so, but this is the first time I use review in github, so maybe I'm wrong.

Couldn't you see these comments?

image

@dchinmay2

Copy link
Copy Markdown
Author

you need to press the "finish review" button

Comment thread src/lib.rs
.stderr(Stdio::piped())
.envs(envs)
.spawn();
.current_dir(root_dir)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Is setting current dir necessary? If not set, will anything not work properly? or is it just a good coding practice?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sorry this is an unrelated change I added to my fork, I did this so that rust-analyzer would pick up overrides correctly (i.e. using nightly rust in projects where I did rustup override set nightly)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'll remove this change and put it in a separate PR if you want

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'll remove this change and put it in a separate PR if you want

Sure, please, thanks.

Comment thread lspce-core.el
(lspce--textDocumentIdenfitier (lspce--uri)) :reason 1))))
(lspce--make-willSaveTextDocumentParams))))

(defun lspce--request-textDocument/willSaveWaitUntil ()

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

IMO, we can make two improvements here. 1. Adding a customizable variable to control whether sending this request or not. 2. Is it better to make this 1 customizable too?
WDYT?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sure, I'll add a customizable variable

@zbelial

zbelial commented Mar 27, 2025

Copy link
Copy Markdown
Owner

you need to press the "finish review" button

Yes. I was wrong, haha. Thanks.

Can you see the review messages now?

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