This is the mail archive of the cygwin-apps mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Re: Patch: Setup.exe - search for package


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]