Page MenuHome
Differential D974

Outliner: "Show Active" now also expands all parents to show the active object itself
AbandonedPublic
Actions

Authored by Julian Eisel (Severin) on Jan 6 2015, 11:16 PM.

Details

Reviewers
Philipp Oeser (lichtwerk)
Maniphest Tasks
T42716: Outliner: "Show Active" now also expands all parents to show the active object itself

Diff Detail

Event Timeline

Philipp Oeser (lichtwerk) updated this revision to Diff 3106.Jan 6 2015, 11:16 PM
Philipp Oeser (lichtwerk) retitled this revision from to Outliner: "Show Active" now also expands all parents to show the active object itself.
Philipp Oeser (lichtwerk) updated this object.
Philipp Oeser (lichtwerk) mentioned this in T42716: Outliner: "Show Active" now also expands all parents to show the active object itself.Jan 6 2015, 11:27 PM
Philipp Oeser (lichtwerk) added a task: T42716: Outliner: "Show Active" now also expands all parents to show the active object itself.
Julian Eisel (Severin) added a reviewer: Julian Eisel (Severin).Jan 6 2015, 11:39 PM
Julian Eisel (Severin) requested changes to this revision.EditedJan 11 2015, 7:27 PM
Julian Eisel (Severin) edited edge metadata.

Had a look on this a few days ago, but didn't find any serious concerns.
I added inline comments to note a few minor things I'd like to see changed. One more picky concern I have, is the location of outliner_open_back, outliner_set_coordinates_element, and outliner_set_coordinates. These should either become "real" utility functions that are located in an own outliner_utility.c file (or at least in an own utility block), or they should just be used locally meaning they should be right above outliner_show_active_exec. Since those functions are only called from outliner_show_active_exec and I currently don't see a need to make this reusable, we should go with the second option IMO. So let's just place them above outliner_show_active_exec.

Other than those concerns, mostly looking good to me.

source/blender/editors/space_outliner/outliner_edit.c
76

SpaceOopos *soops is unused and can be deleted

91

Better have this called outliner_set_coordinates_element_recursive so you can easily see that this is a recursive function.

106

Unnecessary blank line

117

Would prefer to just use 0 instead of startx.

120

Same here, unnecessary blank line

This revision now requires changes to proceed.Jan 11 2015, 7:27 PM
Philipp Oeser (lichtwerk) updated this revision to Diff 3126.Jan 12 2015, 5:54 PM
Philipp Oeser (lichtwerk) edited edge metadata.

Thanx for having a look! (issues mentioned in review should be adressed now...)

Julian Eisel (Severin) accepted this revision.Jan 20 2015, 5:08 PM
Julian Eisel (Severin) edited edge metadata.

LGTM, will check over it again and commit then

This revision is now accepted and ready to land.Jan 20 2015, 5:08 PM
Julian Eisel (Severin) mentioned this in rB3356f3f0bb15: Outliner: "Show Active" - if active is a child, open tree to show child.Jan 20 2015, 5:55 PM
Julian Eisel (Severin) commandeered this revision.Jan 20 2015, 5:57 PM
Julian Eisel (Severin) edited reviewers, added: Philipp Oeser (lichtwerk); removed: Julian Eisel (Severin).
This revision now requires review to proceed.Jan 20 2015, 5:57 PM
Julian Eisel (Severin) abandoned this revision.Jan 20 2015, 5:57 PM

Closed by commit 3356f3f0bb1

Thanks @Philipp Oeser (lichtwerk)!

Julian Eisel (Severin) mentioned this in rBS3356f3f0bb15: Outliner: "Show Active" - if active is a child, open tree to show child.Mar 25 2015, 12:21 PM

Revision Contents
Changeset List

PathSize
source/
blender/
editors/
space_outliner/
97 lines

Diff 3126

View Options

source/blender/editors/space_outliner/outliner_edit.c

Loading...