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 autoreload crash / error #1698

Merged
merged 1 commit into from Jan 21, 2024
Merged

Fix autoreload crash / error #1698

merged 1 commit into from Jan 21, 2024

Conversation

vqns
Copy link
Contributor

@vqns vqns commented Jan 10, 2024

the autoreload plugin may cause errors or crashes when a doc with a (absolute) filename is no longer backed by an actual on disk file.

steps to reproduce:

  1. have lite-xl with a file open in a doc
  2. delete the file, keep the doc open
  3. create a folder with the same name as the file

causes an error if done from within lite-xl (e.g the treeview), a crash if done externally

The deeper issue is that docs keep their filename even when their backing files are deleted, however as far as i can make sense of the dirwatch, there doesn't seem to be a way to tell apart a rename from a deletion + creation. So clearing filenames would also make externally renamed docs dangling (which may be desired behavior?)

See:
https://github.com/lite-xl/lite-xl/blob/master/data/core/init.lua#L199

@takase1121 takase1121 added this to PR in Bugfix Release Tracker via automation Jan 17, 2024
@takase1121 takase1121 moved this from PR to Freeze in Bugfix Release Tracker Jan 17, 2024
@takase1121
Copy link
Member

I wasn't able to replicate this on Windows.

@FijiHasGithub
Copy link
Contributor

Can reproduce both the error and the crash on Alpine Linux 6.6.12, using Lite-XL built from the master branch

@Guldoman
Copy link
Member

Can reproduce on Arch too.
I don't know if this should be fixed here or in the backend.
@adamharrison might have some ideas.

@vqns
Copy link
Contributor Author

vqns commented Jan 20, 2024

I don't know if this should be fixed here or in the backend.

yeah this is just a bandaid fix; the issue is certainly bound to arise in other situations. That said, I don't think it hurts to check that the file info is about an actual file rather than blindly reloading the doc

@takase1121
Copy link
Member

The issue is moved to #1707, this PR should be merged as a hotfix.

@Guldoman Guldoman merged commit c561eb7 into lite-xl:master Jan 21, 2024
10 checks passed
Bugfix Release Tracker automation moved this from Freeze to TODO Jan 21, 2024
takase1121 pushed a commit to takase1121/lite-xl that referenced this pull request Jan 21, 2024
@vqns vqns deleted the fix-autoreload branch March 8, 2024 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants