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

fix on save #4

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

patrickfatrick
Copy link

Adds a "fix on save" preference for workspace and global configuration.

@patrickfatrick
Copy link
Author

@nlydv Can you check this PR out? It's been open for a month.

@nlydv
Copy link
Owner

nlydv commented Jan 8, 2023

@patrickfatrick Hey, sorry I was out of the country last month for the holidays and I guess this got marked as read in my notifications. So thanks for the reminder!

I'll dig into this over the next couple days or so and get back to you.

Thanks for opening a PR by the way!

@nlydv
Copy link
Owner

nlydv commented Jan 13, 2023

Hey @patrickfatrick, so after reading through the changes, everything was looking good to me, and as long as it built/ran as expected I was ready to merge it... but I ran into two issues:

  1. The current Rollup build system isn't configured to bundle external node_module dependencies (just because I haven't needed to, plus I'm not that familiar with Rollup), so I wasn't able to build this PR initially. This isn't a huge deal, the logic provided by the imported nova-extension-utils function was pretty simple to implement directly.

  2. After building and running the PR, I ran into some really confusing behavior related to Nova's callback listener APIs. The fixing on save feature seems to work fine on the first couple of tests, but every time the fixOnSave config (local or global) gets changed, the extension becomes more and more likely to ignore the config that's actually set. In other words if you change the config enough times, it apparently just fixes on save no matter what. I've created a separate branch with some minor changes in order to try and debug this by logging events to the console:

extension console output

This test was done on Nova version 10.6

After every config change, I saved an open SCSS file once. Why or how certain callbacks start getting called several time simultaneously after some time is beyond me. I've tried debugging/reimplementing/working around this from several angles but couldn't find any explain or fix; I tried poking holes in your your code but it seems totally logical.

I'm not totally surprised by this though, I've run into a lot inconsistencies and (presumably) buggy behavior with Nova's APIs related to callback listeners and Disposable objects not fully relinquishing resources or killing processes when disposed. I've actually been thinking about porting much of the logic in this extension into a Rust executable for that reason, among others.

Anyways, maybe you can try and reproduce this by building the branch linked above and see if you're able to find some solution.

@patrickfatrick
Copy link
Author

@nlydv I will take a look! Thanks for checking it out.

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.

None yet

2 participants