← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Eeschema: add wildcard and regex support to component chooser

 

On 19 December 2015 at 16:32, Chris Pavlina <pavlina.chris@xxxxxxxxx> wrote:
> I'm fine with this one, and it all works quite well so I'm assuming Henner
> considers it final. I'd commit it.

Yes, considered final, ready to submit.

(And yes, sorry if I screwed up code styles, I try to be more careful.
Too many projects with different styles to juggle)

-h

>
> On Dec 19, 2015 7:21 PM, "Wayne Stambaugh" <stambaughw@xxxxxxxxx> wrote:
>>
>> On 12/19/2015 2:57 AM, Henner Zeller wrote:
>> > On 18 December 2015 at 21:57, Henner Zeller <h.zeller@xxxxxxx> wrote:
>> >> On 18 December 2015 at 21:50, Chris Pavlina <pavlina.chris@xxxxxxxxx>
>> >> wrote:
>> >>> On Fri, Dec 18, 2015 at 09:40:29PM -0800, Henner Zeller wrote:
>> >>>> On 18 December 2015 at 21:33, Chris Pavlina <pavlina.chris@xxxxxxxxx>
>> >>>> wrote:
>> >>>>> I'm not going to argue about search methods, it'll suffice to say I
>> >>>>> disagree. I don't _want_ to search my library your way, and yes, I
>> >>>>> understand that it can work.
>> >>>>>
>> >>>>> I'm unconvinced that adding one option box is excessive complexity.
>> >>>>> It's
>> >>>>> not in the main UI, it's in the preferences dialog. What's wrong
>> >>>>> with
>> >>>>> preferences? Users can ignore those. Most do. Though the preferences
>> >>>>> dialog does need to be reorganized.
>> >>>>
>> >>>> The problem is, that people don't find it. They get frustrated
>> >>>> because
>> >>>> they want to search with regular expressions, but it doesn't work.
>> >>>> And
>> >>>> they never find the option to set it.
>> >>>
>> >>> Somehow I don't think that wildcard/regex search is going to be a big
>> >>> advertised feature that all the kids will be flocking to kicad for. I
>> >>> also doubt that the sort of person who would want it is the sort of
>> >>> person who couldn't be bothered to look at the options.
>> >>>
>> >>>>
>> >>>> If the dialog would do all matches in parallel and increases scores
>> >>>> for each matcher that triggers, then you get all the simple matching
>> >>>> I
>> >>>> am proposing and all the extended regular expression searching that
>> >>>> you want. And it will work automatically without anyone ever setting
>> >>>> an option.
>> >>>
>> >>> Yes, and clutters up the results with false matches, just like
>> >>> searching
>> >>> for "ATXMEGA*D3" by typing "ATXMEGA D3" does.
>> >>
>> >> But if you have all scoring functions engaged, the relevant come first.
>> >>
>> >>>
>> >>> Seriously, what is it with this mailing list? Every time someone
>> >>> suggests a simple idea, everyone immediately piles on with their
>> >>> favorite way to overcomplicate it.
>> >>
>> >> Calm down. I make suggestions to _simplify_, not complicate.
>> >>
>> >>>
>> >>> I just suggested adding this because I thought it'd be a simple
>> >>> addition
>> >>> that might add some utility for some people. Never imagined wildcards
>> >>> could be so controversial.
>> >>
>> >> As I said, I like wildcards and regexp and like to see them supported.
>> >> But I suggested way to support them automatically.
>> >>
>> >>> Take it or leave it, I'm not going to keep
>> >>> revising it to make it more complex.
>> >>
>> >> You don't have to. I can do that.
>> >
>> > Attached, the simplified patch, using your new matchers:
>> >
>> >    * Uses all your matchers, but does not need to have the factory.
>> >    * No need for extra code to add option for users.
>> >    * Same effect. People can use any syntax they want and get matches
>> > accordingly.
>> >    * It is very unlikely to get unwanted results.
>> >    * Less code (366 vs 833 loc)
>> >    * More user friendly to use (no search for obscure options, if
>> > someone uses FOO*BAR or a regexp it just works as expected).
>>
>> Is this the patch that should be committed?  I took a look at it and I
>> like it.  Nice work gentlemen.  One small comment.  Watch your curly
>> brace placement.  They should always go on the next line not after a
>> statement.
>>
>> >
>> > -h
>> >
>> >>
>> >>>
>> >>>>
>> >>>> -h
>> >>>>
>> >>>>>
>> >>>>>
>> >>>>> On Fri, Dec 18, 2015 at 09:19:54PM -0800, Henner Zeller wrote:
>> >>>>>> On 18 December 2015 at 20:16, Chris Pavlina
>> >>>>>> <pavlina.chris@xxxxxxxxx> wrote:
>> >>>>>>> As discussed earlier today, this patch adds support for both
>> >>>>>>> wildcard
>> >>>>>>> and regular expression search to the eeschema component chooser.
>> >>>>>>> An
>> >>>>>>> option is added to eeschema options to select the "Component
>> >>>>>>> search
>> >>>>>>> method", which defaults to "Simple" (the original behavior).
>> >>>>>>> Regular
>> >>>>>>> expression search was added as a "bonus", as it was used as a
>> >>>>>>> stepping
>> >>>>>>> stone to wildcard search.
>> >>>>>>
>> >>>>>> Cool, I'll try it.
>> >>>>>>
>> >>>>>> BUT: I am skeptical I must admit, as adding these features that are
>> >>>>>> undoublty more complicated for the non-software engineer to use and
>> >>>>>> also to choose from. More choices for the user to handle, and I am
>> >>>>>> not
>> >>>>>> convinced yet that it is worth it.
>> >>>>>>
>> >>>>>> I would rather try to make the search as best as possible that a
>> >>>>>> few
>> >>>>>> words will bring the result. If it doesn't, then the scoring
>> >>>>>> function
>> >>>>>> needs to be tweaked. I am a big fan of simple user interfaces...
>> >>>>>>
>> >>>>>> Do you have some real world examples that are really hard to solve
>> >>>>>> with simple string-matching and scoring of the result as it is now
>> >>>>>> (the XMEGA example in the other thread can be done by just having a
>> >>>>>> space between the xmega and d3, so this is not really a good
>> >>>>>> example).
>> >>>>>>
>> >>>>>> Having simple user interfaces is hugely important IMHO, so we
>> >>>>>> should
>> >>>>>> add features and confusing user options only if the simple way of
>> >>>>>> dealing with it can't do it.
>> >>>>>>
>> >>>>>> -h
>> >>>>>>
>> >>>>>>>
>> >>>>>>> The existing behavior of Henner's component chooser wasn't
>> >>>>>>> changed, just
>> >>>>>>> the matching function it uses.
>> >>>>>>>
>> >>>>>>> Following Wayne's suggestion, an EDA_PATTERN_MATCH base class is
>> >>>>>>> defined
>> >>>>>>> in include/eda_pattern_match.h, and then:
>> >>>>>>>
>> >>>>>>> - EDA_PATTERN_MATCH_SUBSTR implements EDA_PATTERN_MATCH with the
>> >>>>>>> old
>> >>>>>>>   behavior
>> >>>>>>>
>> >>>>>>> - EDA_PATTERN_MATCH_REGEX implements EDA_PATTERN_MATCH providing
>> >>>>>>> regex
>> >>>>>>>   search via wxRegEx and wxRE_ADVANCED.
>> >>>>>>>
>> >>>>>>> - EDA_PATTERN_MATCH_WILDCARD extends EDA_PATTERN_MATCH_REGEX,
>> >>>>>>>   translating a wildcard string to a regular expression and then
>> >>>>>>> loading
>> >>>>>>>   it into the latter. This allows the nice, fast,
>> >>>>>>> wxString-compatible,
>> >>>>>>>   and well tested regex search to be reused for wildcard search.
>> >>>>>>>
>> >>>>>>> --
>> >>>>>>> Chris
>> >>>>>>>
>> >>>>>>> _______________________________________________
>> >>>>>>> Mailing list: https://launchpad.net/~kicad-developers
>> >>>>>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>> >>>>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>> >>>>>>> More help   : https://help.launchpad.net/ListHelp
>> >>>>>>>
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> _______________________________________________
>> >>>>>>> Mailing list: https://launchpad.net/~kicad-developers
>> >>>>>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>> >>>>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>> >>>>>>> More help   : https://help.launchpad.net/ListHelp
>>
>>
>> _______________________________________________
>> Mailing list: https://launchpad.net/~kicad-developers
>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
>
>
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>


Follow ups

References