Generally single entry controls will contain a colon in the label but entire sections will not - thus "Output directory" and "Default line thickness (mm)" should have colons, but "Messages" and "Page Size" should not. This is because the single entry controls read as a single line of text, where it's correct to separate a label with a colon, whereas the section headings read as standalone labels that sit apart from the thing they label.
On Fri, Jul 21, 2017 at 05:41:59PM +0200, Fabrizio Tappero wrote: > Hi Wayne, > > this is an exmaple of the current plot schematic window. > > [image: Inline image 1] > > > I think it is expected that "Output directory:" and "Default line thickness > (mm):" and "Messages:" have a colon. > > Am I correct? > > cheers > Fabrizio > > > > On Fri, Jul 21, 2017 at 4:23 PM, Wayne Stambaugh <stambau...@gmail.com> > wrote: > > > Fabrizio, > > > > I pushed your patch to the master branch. Thank you for your > > contribution to KiCad. > > > > This is not aimed at your patch but one thing I notice is that there is > > and inconsistency in some of our wxStaticBoxSizer() strings. There are > > a few cases where the string ends with a colon (:) but it's rare. For > > the sake of consistency, if you find any cases in the future where this > > is true, please remove the trailing colon. > > > > Cheers, > > > > Wayne > > > > > > On 7/20/2017 5:59 AM, Fabrizio Tappero wrote: > > > Hi Wayne, > > > you are right. Sorry about that. I have reviewed and changed all > > instances. > > > > > > In attachment the new patch. > > > > > > cheers > > > Fabrizio > > > > > > > > > On Wed, Jul 19, 2017 at 3:59 PM, Wayne Stambaugh <stambau...@gmail.com > > > <mailto:stambau...@gmail.com>> wrote: > > > > > > Fabrizio, > > > > > > You are still using header instead of sentence capitalization of text > > > box labels. For example, in pcbnew/dialogs/dialog_plot_base.cpp you > > > changed: > > > > > > "Plot format:" > > > > > > to > > > > > > "Plot Format:" > > > > > > The original capitalization is correct. I see a lot of this in this > > > patch. > > > > > > Please correct the text box label capitalization and resubmit your > > > patch. Also, please give your patches a unique name so I don't have > > to. > > > Even easier, just use the name generated by `git format-patch`. > > > > > > Wayne > > > > > > On 7/19/2017 6:55 AM, Fabrizio Tappero wrote: > > > > Hi Wayne, > > > > really tedious work to spot these kind of mistakes... I hope I > > fixed all > > > > mistakes. > > > > > > > > In attachment the new patch. > > > > > > > > cheers > > > > Fabrizio > > > > > > > > > > > > On Wed, Jul 19, 2017 at 12:19 PM, Fabrizio Tappero > > > > <fabrizio.tapp...@gmail.com <mailto:fabrizio.tapp...@gmail.com> > > > <mailto:fabrizio.tapp...@gmail.com > > > <mailto:fabrizio.tapp...@gmail.com>>> wrote: > > > > > > > > Hi Wayne, > > > > apologies for that. I sometimes think that a unique patch is > > more > > > > friendly than several ones. > > > > > > > > Will fix and resubmit. > > > > > > > > cheers > > > > Fabrizio > > > > > > > > > > > > On Tue, Jul 18, 2017 at 3:35 PM, Wayne Stambaugh > > > > <stambau...@gmail.com <mailto:stambau...@gmail.com> > > > <mailto:stambau...@gmail.com <mailto:stambau...@gmail.com>>> wrote: > > > > > > > > Fabrizio, > > > > > > > > This patch has far more changes than the plot dialogs > > > mentioned > > > > in your > > > > commit message. In the future, please either make your > > commit > > > > message > > > > describe all of the changes related to the patch. I would > > > > prefer that > > > > you only make one wxFormbuilder dialog change per patch. > > > > wxFormbuilder > > > > changes tend to be noisy so it takes longer to review them. > > > > > > > > You are using the incorrect capitalization in text box > > labels. > > > > Text box > > > > labels use sentence capitalization not header > > > capitalization per > > > > the UI > > > > guidelines[1]. > > > > > > > > Please fix your patch accordingly so I can commit it. > > > > > > > > Cheers, > > > > > > > > Wayne > > > > > > > > > > > [1]:http://docs.kicad-pcb.org/doxygen/md_Documentation_ > > development_ui-policy.html > > > <http://docs.kicad-pcb.org/doxygen/md_Documentation_ > > development_ui-policy.html> > > > > > > > <http://docs.kicad-pcb.org/doxygen/md_Documentation_ > > development_ui-policy.html > > > <http://docs.kicad-pcb.org/doxygen/md_Documentation_ > > development_ui-policy.html>> > > > > > > > > On 7/3/2017 4:45 AM, Fabrizio Tappero wrote: > > > > > Hi JP, > > > > > thanks for it. I updated my wxformbuilder to the latest > > > (3.6v) > > > > > version. And I fixed the (mm) issue you mentioned. > > > > > > > > > > In attachment the patch. > > > > > > > > > > thank you > > > > > Fabrizio > > > > > > > > > > > > > > > On Sat, Jul 1, 2017 at 8:40 PM, jp charras > > > > <jp.char...@wanadoo.fr <mailto:jp.char...@wanadoo.fr> > > > <mailto:jp.char...@wanadoo.fr <mailto:jp.char...@wanadoo.fr>>> > > wrote: > > > > >> Le 30/06/2017 à 15:34, Fabrizio Tappero a écrit : > > > > >>> Hello, > > > > >>> in attachment the patch reviewed after Wayne comments. > > > > >>> > > > > >>> Best regards > > > > >>> Fabrizio > > > > >>> > > > > >> > > > > >> Hi Fabrizio, > > > > >> > > > > >> I had a look at your last patch, and found a few issues: > > > > >> for instance the change: > > > > >> > > > > >> - m_defaultLineWidthTitle = new wxStaticText( > > > > sbSizerPlotFormat->GetStaticBox(), wxID_ANY, > > > > >> _("Default line thickness"), wxDefaultPosition, > > > > wxDefaultSize, 0 ); > > > > >> + m_defaultLineWidthTitle = new wxStaticText( > > > > sbSizerPlotFormat->GetStaticBox(), wxID_ANY, _("Line > > > > >> Thickness (mm):"), wxDefaultPosition, wxDefaultSize, 0 > > ); > > > > >> > > > > >> > > > > >> is not good for 2 reasons: > > > > >> - "Default line thickness" is the right title (see a > > > previous > > > > Wayne's answer) > > > > >> - "Line Thickness (mm):" is incorrect: > > > > >> the units symbol " (mm):" is added a run time, because > > > units > > > > can be mm or inches. > > > > >> so never change titles like "Value" to "Value (mm):". > > > > >> It creates a wrong title. > > > > >> > > > > >> Besides your wxFormBuilder version is rather old and > > > creates > > > > incorrect C++ sources: please update it. > > > > >> > > > > >> Thanks. > > > > >> > > > > >> -- > > > > >> Jean-Pierre CHARRAS > > > > >> > > > > >> > > > > >> _______________________________________________ > > > > >> Mailing list: https://launchpad.net/~kicad-developers > > > <https://launchpad.net/~kicad-developers> > > > > <https://launchpad.net/~kicad-developers > > > <https://launchpad.net/~kicad-developers>> > > > > >> Post to : kicad-developers@lists.launchpad.net > > > <mailto:kicad-developers@lists.launchpad.net> > > > > <mailto:kicad-developers@lists.launchpad.net > > > <mailto:kicad-developers@lists.launchpad.net>> > > > > >> Unsubscribe : https://launchpad.net/~kicad-developers > > > <https://launchpad.net/~kicad-developers> > > > > <https://launchpad.net/~kicad-developers > > > <https://launchpad.net/~kicad-developers>> > > > > >> More help : https://help.launchpad.net/ListHelp > > > <https://help.launchpad.net/ListHelp> > > > > <https://help.launchpad.net/ListHelp > > > <https://help.launchpad.net/ListHelp>> > > > > > > > > _______________________________________________ > > > > Mailing list: https://launchpad.net/~kicad-developers > > > <https://launchpad.net/~kicad-developers> > > > > <https://launchpad.net/~kicad-developers > > > <https://launchpad.net/~kicad-developers>> > > > > Post to : kicad-developers@lists.launchpad.net > > > <mailto:kicad-developers@lists.launchpad.net> > > > > <mailto:kicad-developers@lists.launchpad.net > > > <mailto:kicad-developers@lists.launchpad.net>> > > > > Unsubscribe : https://launchpad.net/~kicad-developers > > > <https://launchpad.net/~kicad-developers> > > > > <https://launchpad.net/~kicad-developers > > > <https://launchpad.net/~kicad-developers>> > > > > More help : https://help.launchpad.net/ListHelp > > > <https://help.launchpad.net/ListHelp> > > > > <https://help.launchpad.net/ListHelp > > > <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
