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

Export selection potential fix #1201

Closed
wants to merge 2 commits into from

Conversation

Doublonmousse
Copy link
Collaborator

make export selection match the other output modes

There is a minute difference.
On the two other modes we have
if let Some(file)
then we call span_future_local

This was the reverse for the selection export.
Trying to see if this fixes #1199.
I've added a few debug trace to help with debugging for this issue

(it will only be a style PR otherwise)

There is a minute difference.
On the two other modes we have
`if let Some(file)`
then we call `span_future_local`

This was the reverse for the selection export.
Trying to see if this fixes flxzt#1199 (will only be a style PR otherwise)
@Doublonmousse Doublonmousse marked this pull request as ready for review September 11, 2024 15:48
@Doublonmousse
Copy link
Collaborator Author

This needs to be rewritten to the new glib::clone syntax. Maybe things would work with the new glib version but in the current state, this was rewritten to the new syntax with potentially the same bug.

The fix was confirmed to work.

@flxzt
Copy link
Owner

flxzt commented Sep 20, 2024

isn't the actual fix to change the reference taken by the closure either to a strong reference here and completely remove taking a reference here and move ownership of the selected file into the confirm button callback? I assume saving fails silently because upgrading the reference fails

@flxzt
Copy link
Owner

flxzt commented Sep 20, 2024

I'll follow with a fix and some code cleanup and improved consistency

@Doublonmousse
Copy link
Collaborator Author

The first clone works but not the second one (with the current code).

Honestly, I'm half guessing, for me this was an issue on selected_file being a weak reference and getting dropped (ref count to 0) before the second clone is called.

I think that's similar to what you're proposing, I kinda went with what was done elsewhere (even though I code in rust I can't parse a sentence with two into :))

@flxzt
Copy link
Owner

flxzt commented Sep 20, 2024

my bad that wasn't very clear. take a look at #1227 's code which should do the same thing logically

@flxzt flxzt closed this in #1227 Sep 28, 2024
@Doublonmousse Doublonmousse deleted the export_selection branch September 28, 2024 07:23
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.

exporting selection doesn't work
2 participants