AnWeber / vscode-statusbar-command

MIT License
16 stars 4 forks source link

Allow script to be read from file #22

Closed dandavison closed 2 years ago

dandavison commented 2 years ago

Hi @AnWeber, do you have any sympathy for this feature?

My current use case is displaying information about the git repo I'm currently working in (it's Scala, and I'm displaying information about the "build server" project that the current file may or may not be part of.) Specifically, if vscode.window.activeTextEditor.document.uri.path matches a certain pattern, then I use that pattern to locate a certain symlink, and display the file name that the symlink is pointing to using fs.readlinkSync. That's a bit too much code complexity to fit in one line inside a string in settings.json. (I did consider a hack using eval and readFileSync to avoid asking you for this feature, but require didn't work in the eval).

If you are willing to add this, would you mind helping me make it compatible with your compile-web target? It seems that we would need to make this feature conditional on the platform?

AnWeber commented 2 years ago

I have also come across this problem. It would also be possible to solve this using require. But this would cause problems through NodeJS caching. It would still be worth considering adding a watcher on the ScriptFile, so that when changes are made to the file, the StatusbarItem is updated (see VSCode API - createFileSystemWatcher. This would not happen currently and would confuse. And I like to always forget that. Thank you, I think it's great when someone looks at other people's code and improves it.

dandavison commented 2 years ago

Thanks for reviewing this! I've added a watcher to the scriptFile, and I will continue to test that change locally.

Meanwhile, do you have any thoughts on how to modify things so that the compile-web target functions despite my usage of fs, which is not available on web?

AnWeber commented 2 years ago

You can use vscode.workspace.fs to access fileSystem in a way which is supported in browser environment. I added a commit to your branch. I have now tested it a bit and I think it works correctly, so I'll merge it right now

dandavison commented 2 years ago

Thank you! I see you've converted several functions to async: I should have said that that needed addressing before merge! I was testing with one script file that was doing a slow operation and it seemed to lock up the thread that was accepting keyboard input in VSCode, although not mouse events. I didn't realize VSCode architecture was such that that would happen but in any case thanks a lot for finalizing and merging this.

AnWeber commented 2 years ago

@dandavison If the script should also support async, I could gladly add that. In general, the editor should not be frozen by the script, because VSCode runs all extensions in a separate thread. Only these are slowed down by this.

image
dandavison commented 2 years ago

If the script should also support async, I could gladly add that.

Yes, I think that would be helpful. I would also like the ability to put a StatusbarCommand into an "in-progress" state, for example using … or 🟠. I.e. it would start showing that content when the script starts, and then switch to show the final display when the script finishes.

In general, the editor should not be frozen by the script, because VSCode runs all extensions in a separate thread.

OK, I'll watch out for the behavior again and see if I can understand why keyboard input became frozen. It may have been another misbehaving process on my machine.

AnWeber commented 2 years ago

v2.50. added support for using await in script content

dandavison commented 2 years ago

Hi @AnWeber, I'm experiencing a bug where multiple instances of statusbar commands are populating the status bar:

image

I haven't tracked the bug down yet but I assume it's a problem with my PR so wanted to warn you.

AnWeber commented 2 years ago

@dandavison Your change is not the cause for the duplication. I encountered the error myself, but could not reproduce it myself. It seems something with my update/ dispose logic is broken, but I could not find the loophole till now.

dandavison commented 1 year ago

Any luck finding out what's causing the duplicates? Have you continued to experience the problem?

AnWeber commented 1 year ago

No, didn't get it out. It only happens to me when I change the settings file, so it hasn't been a high priority for me yet.