kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #30099
Re: [PATCH] correct text inside two importantplot windows
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>>
>>>> >
>>>> >
>>>> >
>>>>
>>>>
>>>
>>>
>
>
Follow ups
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