Fix T84327: outliner_id_copy_tag was only copying from expanded nodes
ClosedPublic
Actions

Authored by Pi Lanningham (Quantumplation) on Jan 5 2021, 2:59 AM.

Details

Reviewers

Brecht Van Lommel (brecht)
Bastien Montagne (mont29)

Maniphest Tasks

T84327: outliner.id_copy/paste doesn't work with hierarchy if it not expanded

Commits

rBf0ae0f490eaa: Fix T84327: outliner_id_copy_tag was only copying from expanded nodes.

Summary

If a node was closed in the hierarchy, we would only copy that node,
even if child nodes were selected.

This restriction seems intentional, but it's unclear why. The most
intuitive behavior is to copy all items selected, regardless of whether
the node is collapsed.

By removing the if condition, we always recurse, grabbing anything the
user has selected for the copy. This method doesn't appear to be used
anywhere else so I don't think this would have any unintended consequences.

I believe another potential solution would be to set the
space_outliner->search_flags to include SO_SEARCH_RECURSIVE, as the
TSELEM_OPEN macro checks this, but given that I'm not as familiar with
the code base, I went for the more straightforward approach.

Diff Detail

Repository: rB Blender

Event Timeline

Pi Lanningham (Quantumplation) requested review of this revision.Jan 5 2021, 2:59 AM

Pi Lanningham (Quantumplation) created this revision.

Harbormaster completed remote builds in B11984: Diff 32440.Jan 5 2021, 2:59 AM

Pi Lanningham (Quantumplation) added a project: User Interface.Jan 5 2021, 3:06 AM

Note: the original commit where this was added doesn't provide much insight as to why this choice was made:

https://developer.blender.org/rB94388d69bb8082945e95f1b7583320cd43d90c69

Pi Lanningham (Quantumplation) added reviewers: Brecht Van Lommel (brecht), Dalai Felinto (dfelinto), Nathan Craddock (natecraddock).Jan 5 2021, 3:14 AM

Pi Lanningham (Quantumplation) retitled this revision from Recurse unconditionally in outliner_id_copy_tag, See T84327 to Fix T84327: outliner_id_copy_tag was only copying from expanded nodes.Jan 5 2021, 4:14 AM

Eugene Du (APEC) awarded a token.Jan 5 2021, 9:19 AM

RedMser (RedMser) added a subscriber: RedMser (RedMser).Jan 7 2021, 1:27 AM

Would it be possible to add an option to copy the node and all its children regardless of selection?
It feels wrong to have to select all the children first when most of the time you want to copy the whole subtree

In D9995#250839, @John Cox (theois) wrote:

Would it be possible to add an option to copy the node and all its children regardless of selection?
It feels wrong to have to select all the children first when most of the time you want to copy the whole subtree

Hello,
write me PM on blenderartists.org or devtalk.blender.org (I'm "APEC" here), maybe I have a solution for you.

In D9995#250839, @John Cox (theois) wrote:

Would it be possible to add an option to copy the node and all its children regardless of selection?
It feels wrong to have to select all the children first when most of the time you want to copy the whole subtree

Probably! But I think that's a separate feature request, and a bit outside the scope of this particular bugfix.

Brecht Van Lommel (brecht) accepted this revision.Jan 11 2021, 10:17 AM

This revision is now accepted and ready to land.Jan 11 2021, 10:17 AM

I imagine this was just a copy-paste error, in another operator ignoring hidden items makes sense, but not here.

But I'm not entirely sure, so checking with @Bastien Montagne (mont29) who implemented this operator.

This revision now requires review to proceed.Jan 11 2021, 11:18 AM

yes indeed, in that context think it makes sense to copy also non-visible-in-outliner objects...

This revision is now accepted and ready to land.Jan 11 2021, 11:58 AM

Closed by commit rBf0ae0f490eaa: Fix T84327: outliner_id_copy_tag was only copying from expanded nodes. (authored by Pi Lanningham (Quantumplation), committed by Bastien Montagne (mont29)). · Explain WhyJan 11 2021, 12:05 PM

This revision was automatically updated to reflect the committed changes.

Bastien Montagne (mont29) added a commit: rBf0ae0f490eaa: Fix T84327: outliner_id_copy_tag was only copying from expanded nodes..

Gilberto Rodrigues (gilberto_rodrigues) awarded a token.Jan 14 2021, 7:11 AM

Revision Contents
Changeset List

Path

Size

source/

blender/

editors/

space_outliner/

outliner_edit.c

4 lines

Diff 32604

View Options

source/blender/editors/space_outliner/outliner_edit.c

Fix T84327: outliner_id_copy_tag was only copying from expanded nodesClosedPublicActions