Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: delete-file-prev not working as intended #726

Merged
merged 1 commit into from
Oct 21, 2023

Conversation

christoph-heinrich
Copy link
Contributor

No description provided.

@tomasklaen
Copy link
Owner

Aren't we running a risk here that state.path will update between navigate_item() and delete_file()? It kind of relies on navigate_item() being implicitly asynchronous, and it also makes the code harder to follow.

Rather save it into a current_path at the top and use that to delete from menu and drive.

@christoph-heinrich
Copy link
Contributor Author

The script runs in a single thread, unless we update it in that code path it can't change until we're done and the property observer can run.

@tomasklaen
Copy link
Owner

Yeah but look at the code flow from a perspective of someone not that familiar with all mpv/uosc internals (or me in a couple years going back trying to maintain it :)). They'll see navigate(), and then us deleting the newly open file, and go "huh?".

@christoph-heinrich
Copy link
Contributor Author

The last line of the original used state.path...

if is_local_file then delete_file(state.path) end

@tomasklaen tomasklaen merged commit d49f385 into tomasklaen:main Oct 21, 2023
@tomasklaen
Copy link
Owner

tomasklaen commented Oct 21, 2023

The last line of the original used state.path...

That was bad too then :) I'm just trying to make the code easy to follow, is all.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants