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

Iterate in tab order for *Close Other Documents* #2347

Open
wants to merge 1 commit into
base: master
from

Conversation

@ntrel
Copy link
Member

commented Oct 8, 2019

Iterate in notebook tab order instead of documents_array order.

@LarsGit223

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2019

What is this PR doing or better said what is the advantage compared to the current code?

@ntrel

This comment has been minimized.

Copy link
Member Author

commented Oct 8, 2019

@LarsGit223 The existing code iterates documents_array sequentially. It's better for the user to iterate in notebook tab order, because tabs can be reordered*. The user will notice the difference when there are unsaved documents to close.

[Edit:] *and documents_array slots can be reused when docs have been closed.

@codebrainz

This comment was marked as resolved.

Copy link
Member

commented Oct 9, 2019

Not sure if it matters, but I believe this will iterate back-to-front for users with RTL UI.

@ntrel

This comment was marked as resolved.

Copy link
Member Author

commented Oct 9, 2019

I believe this will iterate back-to-front for users with RTL UI

OK, looks like Close All has this same issue though:
https://github.com/geany/geany/blob/master/src/document.c#L3372-L3380

I don't think this matters much, at least the order is still simple for the user to understand.

@codebrainz

This comment was marked as resolved.

Copy link
Member

commented Oct 10, 2019

I don't think this matters much, at least the order is still simple for the user to understand.

Agree.

@@ -857,7 +857,6 @@ static void on_openfiles_document_action(GtkMenuItem *menuitem, gpointer user_da
while (i >= 0 && gtk_tree_model_iter_nth_child(model, &child, &iter, i))
{
gtk_tree_model_get(model, &child, DOCUMENTS_DOCUMENT, &doc, -1);

This comment has been minimized.

Copy link
@codebrainz

codebrainz Oct 10, 2019

Member

Should get rid of this change since it's unrelated and in another file.

This comment has been minimized.

Copy link
@codebrainz

codebrainz Oct 10, 2019

Member

Ah, I thought I had deja-vu when writing above comment, it was in #2346 that this came from I guess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.