Seth, I have a few comments on this patch.
First and foremost, you should have gotten input before you spent the time and effort doing this. It may have saved you some grief. I also know there is a tendency to make a big push to get last minute changes in before the upcoming feature freeze so that is not helping your cause. There are few coding policy issues. Two spaces are required between function definitions in cpp source files. It also preferable to have a empty line before and after control blocks so it's easier to see where they begin and end. Other than that, everything else looks fine. On 11/10/2017 1:50 PM, Seth Hillbrand wrote: > One of the Eeschema features that has been requested for a while is > customizable graphic line styles that allow greater differentiation in > schematic documentation. c.f. > https://bugs.launchpad.net/kicad/+bug/594059 > https://bugs.launchpad.net/kicad/+bug/1405026 > > The limitation has been not wanting to change the existing schematic format. > > I propose a way around this while implementing the desired feature. > Specifically, the attached patch allows the user to customize graphic > lines (schematic wires are disallowed), with the extra wire data being > stored at the end of the wire line. This change should be transparent > to the existing schematic format as the trailing characters are ignored > in the legacy file reader. Thus customization will be ignored if you > are sharing files between versions. Just because you found a work around in the parser does not mean your change does not constitute a file format change. When someone loads a schematic in an earlier version of eeschema and saves the schematic, all of this information will be lost. That smells like a file format change to me. If I were going to accept this patch, it would require a file version bump. I would also prefer that you didn't use tabs in the file writer. Did you try loading this with stable version 4 which uses the old parser? > > Second, to avoid large, useless diffs in versioning storage, the patch > does not store any additional formatting data unless it has been changed > from the default. This is always a good idea IMO. Creating unnecessary file diffs makes our VCS users happy. > > I'm attaching an image showing the line edit dialog as well as the patch > implementing this. Please let me know if there are any questions or > suggestions for improvement. The dialog alignment could use some work but overall it looks good. If you make the changes I suggested and no one else strongly objects, I would be willing to permit this change even though it goes against my better judgement. Cheers, Wayne > > Best- > Seth > > > _______________________________________________ > Mailing list: https://launchpad.net/~kicad-developers > Post to : kicad-developers@lists.launchpad.net > Unsubscribe : https://launchpad.net/~kicad-developers > More help : https://help.launchpad.net/ListHelp > _______________________________________________ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp