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 sourcemap.sourcemapFile configuration #665

Merged
merged 14 commits into from
Nov 9, 2024

Conversation

TRHeliad
Copy link
Contributor

@TRHeliad TRHeliad commented Jul 1, 2024

Adds an extension configuration for choosing the name of the sourcemap.

There is actually an issue I noticed with unregistering watched files. I am not experienced in working with an LSP so I'm not really sure why the message I am sending to unregister didChangedWatchedFilesCapability isn't actually stopping vscode from sending file changed messages for the sourcemap.

@TRHeliad
Copy link
Contributor Author

@JohnnyMorganz do you know anything about unregistering capabilities and why it might not be working? This isn't really a huge issue because if you reload the workspace after changing config it won't be watching.

@JohnnyMorganz
Copy link
Owner

I'm not too familiar. If you set your logging level to verbose, do you get a response to your unregister capability request?

@TRHeliad TRHeliad deleted the branch JohnnyMorganz:main July 13, 2024 15:09
@TRHeliad TRHeliad closed this Jul 13, 2024
@TRHeliad TRHeliad deleted the main branch July 13, 2024 15:09
@TRHeliad TRHeliad restored the main branch July 13, 2024 15:10
@TRHeliad TRHeliad reopened this Jul 13, 2024
@TRHeliad
Copy link
Contributor Author

Looks like the LSP spec has a little typo so my parameter name was wrong. The unregistration gets a success response now, however vscode still seems to be watching the sourcemap if sourcemaps were enabled by the configuration earlier in the same session. Seems like it is out of my hands, but shouldn't be a big deal since you can reload to window if you don't want it watched.

@TRHeliad TRHeliad marked this pull request as ready for review July 20, 2024 18:59
@TRHeliad
Copy link
Contributor Author

Closes #661

Copy link
Owner

@JohnnyMorganz JohnnyMorganz left a comment

Choose a reason for hiding this comment

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

Thanks, and so sorry for the delay!

Overall looks good to me, I did a slight change to make the ID different that we register for sourcemap watching compared to standard watching. Since we don't want to deregister the default watcher when we actually just want to deregister sourcemap watching.

Do you mind giving a quick check to see if this still works? Otherwise I can merge it in

@TRHeliad
Copy link
Contributor Author

Just pushed a fix I forgot to add. Tested and working for me.

Copy link
Owner

@JohnnyMorganz JohnnyMorganz left a comment

Choose a reason for hiding this comment

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

Thanks!

@JohnnyMorganz JohnnyMorganz enabled auto-merge (squash) November 9, 2024 16:48
@JohnnyMorganz JohnnyMorganz merged commit 846b364 into JohnnyMorganz:main Nov 9, 2024
11 checks passed
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