Patch: Setup.exe - search for package

Andrew Punch andrew@magneticbooks.com.au
Thu Apr 2 23:55:00 GMT 2009


Dave Korn wrote:
> Dave Korn wrote:
>   
>> Andrew Punch wrote:
>>     
>>> Hi,
>>>
>>> I have attached a patch for searching packages in the package selection
>>> screen. The patch is against version 2.573.2.3 - I couldn't get the CVS
>>> head to build due to a libtool version problem.
>>>       
>>   Thanks for contributing this.  I'm currently up-porting it to CVS HEAD and
>> giving it some testing.  None of the other maintainers seem to have any
>> objections, so I'll commit it once I'm sure it works right.  (Your ChangeLog
>> entry isn't in the standard format but I'll fix that up for you.)
>>     
>
>   Ok, it looks pretty good.  Functionality wise, I have a couple of thoughts:
> I think the search box needs a label attached and a tool-tip, and IMO I think
> it might look nicer if it was left-aligned away from the view controls; what
> do you (and everyone else) think?  Secondly, every time you change the
> search-box contents in the category view, it collapses all expanded
> categories; I'd like to avoid that if we can, but haven't yet looked to see how.
> <snip>
>
> .. or here where you've just gone and added incorrect indentation without
> making any changes at all.  You should always match the existing indentation
> style of any code you're working with; if it uses TABs, use TABs, if it uses
> spaces, use spaces.  And above all, watch out if your editor is set to
> auto-convert between the two!
>
> <snip>  

Hi,

Looks like I forget to tidy up a few things before I gave the patch to 
you - oh well developing with an 18 month old running round your 
computer will do that :)

I think a tooltip should be added. The alignment does need to be fixed 
up - either left aligned or flush right against the other controls.

A label sounds good to me. I think the usability benefit of knowing what 
this box does outweighs the slight extra visual noise.

I deliberately decided not to tackle the collapse of categories problem 
because it would probably involve messing with the PickView control. I 
would rather look at that in a separate patch and look at the 
Prev/Curr/Exp collapse problem at the same time.

Thanks for giving some detail on the format you want future patches in. 
I'll also keep an eye on whitespace in the future - particularly where I 
have added some debugging code and then subsequently removed that code.

So would you like me to do the tooltip, alignment and label - or would 
you prefer to do it?

-Andrew



More information about the Cygwin-apps mailing list