Comment 17 for bug 171287

Revision history for this message
su_v (suv-lp) wrote :

Patch tested with Inkscape 0.48+devel r10740 on Mac OS X 10.5.8 (i386):

- As discussed on irc, the option 'Relative to' 'First|Last selected' node requires a major rewrite of internal functions (selection handling (list vs set) in the node tool), and could (hopefully will ;) ) be added later on.

- With regard to the order of the options in the dropdown list: 'Average' as default entry seems best to me (besides having been the (single) default mode in 0.48.x, it applies equally to both horizontal and vertical alignment, whereas the other options (Top Node|Bottom Node|Left Most Node|Right Most Node) might be chosen depending on the alignment task).
After taking a closer look at the options for 'Align nodes', maybe making "Average" the default while keeping it at the bottom would be best after all? For aligning objects, 'Selection' is the default, while being the last option in the list.

- Preferences: AFAICT (with my limited knowledge about C++), you reuse the preferences setting of the 'Relative to' option for objects ("align-to"). IMHO object and node alignment need independent preferences for 'Relative to' because the items in the two lists do not correspond or relate by their order (i.e. the two lists don't or should not share the same last used value).

- Default value: AFAICT, the current default (if the preference 'align-to' doesn't exist yet), is '4' (i.e. "Right Most Node"):

from src/ui/dialog/align-and-distribute.cpp, line 1053:
    _node_anchor_combo.set_active(prefs->getInt("/dialogs/align/align-to", 4));

Shouldn't it either default to '0', or else 'Average' be moved back down to the bottom of the list again (maybe best)? [1]

- Compiler warning is still present (GCC 4.2.1):
  CXX ui/dialog/align-and-distribute.o
ui/dialog/align-and-distribute.cpp: In member function ‘Inkscape::UI::MultiPathManipulator::TargetMode Inkscape::UI::Dialog::AlignAndDistribute::getNodeAlignTarget()’:
ui/dialog/align-and-distribute.cpp:1334: warning: control reaches end of non-void function

[1] Sorry for my confusing comments earlier on irc - my mistake: probably the earlier version of the patch did what I actually wanted - only that it reused the value stored in 'align-to' for objects, and thus not default to 'Average'.