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 LSP-file-watcher-rust #121

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

ruihe774
Copy link

Copy link

@STReviewBot STReviewBot left a comment

Choose a reason for hiding this comment

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

Automated testing result: WARNING

Repo link: LSP-file-watcher-rust
Results help

Packages added:
  - LSP-file-watcher-rust

Processing package "LSP-file-watcher-rust"
  - WARNING: '.no-sublime-package' is defined. Please verify that it is *really* necessary
  - WARNING: It looks like you're using platform-dependent code. Make sure you thought about the platform key in your pull request.
    - File: watcher.py
    - Line: 24, Column: 12

@rchl
Copy link
Member

rchl commented Oct 22, 2024

Would you be open to hosting this in the sublimelsp org?

It would make it easy to update things in case we do refactorings or breaking changes.

Also it could use a bit of cleanup in regards to unused code and references to Chokidar.

@ruihe774
Copy link
Author

Would you be open to hosting this in the sublimelsp org?

It would make it easy to update things in case we do refactorings or breaking changes.

Sure.

Also it could use a bit of cleanup in regards to unused code and references to Chokidar.

I do not get you. What is "unused code and references to Chokidar"?

@rchl
Copy link
Member

rchl commented Oct 22, 2024

Would you be open to hosting this in the sublimelsp org?
It would make it easy to update things in case we do refactorings or breaking changes.

Sure.

Nice. I've created repo at https://github.com/sublimelsp/LSP-file-watcher-rust. Can you make a PR against it? I will give you permissions to manage the repo a bit later.

I do not get you. What is "unused code and references to Chokidar"?

Your python code has some unused stuff like TemporaryInstallationMarker and a lot of references to "Chokidar" which this is not.

@ruihe774
Copy link
Author

Your python code has some unused stuff like TemporaryInstallationMarker

I did not notice that. lol maybe I should use a linter.

a lot of references to "Chokidar" which this is not.

I just reuse the name. The built binary is also called chokidar. I don't think it is a problem; "chokidar" is not a trademark, and all these are invisible to end users. Nevertheless, if you think it is inappropriate, I can change the naming.

@rchl
Copy link
Member

rchl commented Oct 22, 2024

I just reuse the name. The built binary is also called chokidar. I don't think it is a problem; "chokidar" is not a trademark, and all these are invisible to end users. Nevertheless, if you think it is inappropriate, I can change the naming.

It's not a blocking issue but that name can get surfaced to the user in some errors so I would still try to clean that up to avoid confusion.

If you don't care too much then I can do that once the code is migrated to sublimelsp.

@ruihe774
Copy link
Author

ruihe774 commented Oct 22, 2024

If you don't care too much then I can do that once the code is migrated to sublimelsp.

Feel free to modify it ❤️. I do not care much about the python part. In fact 99% of the python code is inherited (mindlessly) from the original implementation.

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.

3 participants