kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #30116
Re: [PATCH] correct text inside two importantplot windows
Hi guys,
thanks for the explanation.
cheers
Fabrizio
On Fri, Jul 21, 2017 at 11:06 PM, Wayne Stambaugh <stambaughw@xxxxxxxxx>
wrote:
> This is correct. I just remember if the string in a group box
> (wxStaticBoxSizer) than it doesn't get a colon. If the string is static
> text (wxStaticText) that describes a text control, then it should end in
> a colon.
>
> On 7/21/2017 2:31 PM, Chris Pavlina wrote:
> > 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 <stambaughw@xxxxxxxxx>
> >> 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 <
> stambaughw@xxxxxxxxx
> >>>> <mailto:stambaughw@xxxxxxxxx>> 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.tappero@xxxxxxxxx <mailto:fabrizio.tappero@xxxxxxxxx>
> >>>> <mailto:fabrizio.tappero@xxxxxxxxx
> >>>> <mailto:fabrizio.tappero@xxxxxxxxx>>> 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
> >>>> > <stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>
> >>>> <mailto:stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>>>
> 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.charras@xxxxxxxxxx <mailto:jp.charras@xxxxxxxxxx>
> >>>> <mailto:jp.charras@xxxxxxxxxx <mailto:jp.charras@xxxxxxxxxx>>>
> >>> 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@xxxxxxxxxxxxxxxxxxx
> >>>> <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
> >>>> > <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx
> >>>> <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>>
> >>>> > >> 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@xxxxxxxxxxxxxxxxxxx
> >>>> <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
> >>>> > <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx
> >>>> <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>>
> >>>> > 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>>
> >>>> >
> >>>> >
> >>>> >
> >>>>
> >>>>
> >>>
> >>>
> >
> >
>
>
References
-
Re: [PATCH] correct text inside two importantplot windows
From: Fabrizio Tappero, 2017-06-30
-
Re: [PATCH] correct text inside two importantplot windows
From: jp charras, 2017-07-01
-
Re: [PATCH] correct text inside two importantplot windows
From: Fabrizio Tappero, 2017-07-03
-
Re: [PATCH] correct text inside two importantplot windows
From: Wayne Stambaugh, 2017-07-18
-
Re: [PATCH] correct text inside two importantplot windows
From: Fabrizio Tappero, 2017-07-19
-
Re: [PATCH] correct text inside two importantplot windows
From: Fabrizio Tappero, 2017-07-19
-
Re: [PATCH] correct text inside two importantplot windows
From: Wayne Stambaugh, 2017-07-19
-
Re: [PATCH] correct text inside two importantplot windows
From: Fabrizio Tappero, 2017-07-20
-
Re: [PATCH] correct text inside two importantplot windows
From: Wayne Stambaugh, 2017-07-21
-
Re: [PATCH] correct text inside two importantplot windows
From: Fabrizio Tappero, 2017-07-21
-
Re: [PATCH] correct text inside two importantplot windows
From: Chris Pavlina, 2017-07-21
-
Re: [PATCH] correct text inside two importantplot windows
From: Wayne Stambaugh, 2017-07-21