On 2019-10-14 14:42, Zficani Zficani wrote:
Hi,
No problem, I just wanted to make sure I sent the message properly.
Here's a single squashed patch with all previous changes and these
comments about copying selection.
Thank you so much for your review.
Hi Zficani-
The functionality feels correct and I really like it. Here are a few
comments on the current patch:
1) I would prefer that the disabled options in the Annotation page are
grey (disabled) and not hidden when the option is unchecked. This
reserves the correct space for them when we add options in the future.
2) Please double-check your code formatting. Spaces inside the
parentheses are missing in a few spots.
3) Don't use C-style casts. C++ static_cast<type>() is preferred.
4) Single-line statements after if/else don't get brackets {}
5) I think that pasting Unit B of a component should paste as the
first missing Unit B in the schematic and not the next open annotation
number. See the attached image for the result of duplicating a quad
op-amp for an example of this problem.
This will be a great addition to KiCad. Thank you for taking this one
on!
Best-
Seth
Seth Hillbrand
KiCad Services Corporation
+1 530 302 5483 | +1 212 603 9372
www.kipro-pcb.com
Davis, CA
