Bug 112661 - [Patch] Comments on Apply Patch support and dialog
Summary: [Patch] Comments on Apply Patch support and dialog
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Team (show other bugs)
Version: 3.1   Edit
Hardware: PC Windows XP
: P2 normal (vote)
Target Milestone: 3.2 RC2   Edit
Assignee: Bogdan Gheorghe CLA
QA Contact:
URL:
Whiteboard:
Keywords: polish
: 132693 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-10-14 13:02 EDT by Michael Van Meekeren CLA
Modified: 2006-04-27 12:16 EDT (History)
1 user (show)

See Also:


Attachments
empty space below compare (44.67 KB, image/png)
2005-10-14 13:52 EDT, Michael Van Meekeren CLA
no flags Details
Apply Patch improvements (1.81 KB, patch)
2006-04-12 16:18 EDT, Bogdan Gheorghe CLA
no flags Details | Diff
Updated Apply Patch patch (3.89 KB, patch)
2006-04-13 00:11 EDT, Bogdan Gheorghe CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Van Meekeren CLA 2005-10-14 13:02:29 EDT
I just used the apply patch in build I20051011 and have the following comments:

Open the Team > apply patch dialog with a patch in the clipboard

Page 1
0) open the dialog on an empty clipboard, error says "clipboard does not contain
text" ... see #2, I've not committed any error yet
0b) after #1, if I go and put something in the clipboard, giving focus back to
the dialog should re-check the clipboard contents to see if it is now good as I
might have gone away and copied some text and came back
1) Text on the top refers to "define the resource to patch" but there is nothing
on this patch that let's you do this
2) When choosing the "File" option before entering the file, an error is brought
up right away, this is against guidelines, you can not tell a user they have
made an error before they have done anything
3) Same as #2 for the "Workspace" selection
4) Need a mnemonic on the label above the tree which will cause the tree to have
focus
Page 2
5) Need a mnemonic on the label above the tree which will cause the tree to have
focus
6) Tree should sort folders and files separately, (this has been noted in a
different bug report) it is very frustrating to see things alphabetically with
files and folders mixed together.  (there should be a viewer sorter available
for this from the workbench)
7) If a patch file (or the patch in the clipboard etc..) contains multiple
patches, why can I select files here? Could the wizard just show me folders?
8) if the patch is a single file couldn't the wizard try and select the file for
me for free, it has the path etc... so it should be able to do this, in fact
would it be true that once a project is selected there is always enough
information to make the right selection?

Page 3:
9) type in the text "d" for "Maximum fuzz factor", no warning or error is
provided, what is the fuzz factor, how does it work?  When I press Guess what
happens?
10) Should "Reverse patch" be renamed "Undo patch"?  
11) when I select an item and the source compare opens, there is a lot of wasted
space below it... see attachment
12 [details])typically the compare area is too small and I have to resize the dialog
larger , the dialog should remember this when I use it again
13)sometimes there are little red exclamation icons next to items, but there is
no information provided as to what this means, and how can I fix it?
14) when making the dialog wider there is no need for the "Reverse Patch" and
"Ignore white space" checkboxes to move to the right... they should stick to the
left
Comment 1 Michael Van Meekeren CLA 2005-10-14 13:52:41 EDT
Created attachment 28301 [details]
empty space below compare
Comment 2 Michael Valenta CLA 2005-10-14 14:07:02 EDT
MVM, thanks for the good comments. 

The page 1 comments result from the changes we made and we should address 
these for M3. I think the page 3 comments apply to the old wizard as well. We 
should verify this to make sure we haven't introduced these. If they are old, 
we should log a separate bug report to cover them and address them later in 
3.2, time permitting. If, on the other hand, they are regressions, we need to 
address them for M3.

As for the extra space at the bottom of the wizard, it is caused by the use of 
the wizard progress monitor. I'm not sure there is much we can do about it. I 
think it would be worthwhile to investigate adding the ability to save he 
wizard size and location. I believe a new feature was added by the UI team 
that will simplify this for us. This doesn't fully compensate for the lost 
space, but will be a big help.
Comment 3 Michael Van Meekeren CLA 2005-10-14 15:48:50 EDT
agree fixing #12 would be good for 3.2

Comment 4 Bogdan Gheorghe CLA 2005-10-21 11:03:38 EDT
Submitted a patch in Bug 110481 that takes care of:

>0) open the dialog on an empty clipboard, error says "clipboard does not
>contain text" ... see #2, I've not committed any error yet  

Fix: no more errors on start of dialog

>0b) after #1, if I go and put something in the clipboard, giving focus back to
>the dialog should re-check the clipboard contents to see if it is now good as I
>might have gone away and copied some text and came back

Fix: added listener for shell activation which prompts a check for patch contents

>1) Text on the top refers to "define the resource to patch" but there is
>nothing on this patch that let's you do this

Fix: renamed to "Select the patch location." (which is what it actually does)

>2) When choosing the "File" option before entering the file, an error is
>brought up right away, this is against guidelines, you can not tell a user they
>have made an error before they have done anything

Fix: check is only run when a file path has actually been entered

> 3) Same as #2 for the "Workspace" selection

Fix: ditto

>4) Need a mnemonic on the label above the tree which will cause the tree to
>have focus

Fix: done


Page 2
>5) Need a mnemonic on the label above the tree which will cause the tree to
>have focus

Fix : done

>6) Tree should sort folders and files separately, (this has been noted in a
>different bug report) it is very frustrating to see things alphabetically with
>files and folders mixed together.  (there should be a viewer sorter available
>for this from the workbench)

Fix: changed all trees to use ResourceSorter

Page 3:

>12 [edit])typically the compare area is too small and I have to resize the
>dialog larger , the dialog should remember this when I use it again

Fix: settings persisted
Comment 5 Bogdan Gheorghe CLA 2005-10-21 11:14:52 EDT
Which leaves the following points on the TODO list:

Page 2:

>7) If a patch file (or the patch in the clipboard etc..) contains multiple
>patches, why can I select files here? Could the wizard just show me folders?

You can only get to the second page if the patch is a single select patch -
otherwise the user gets taken automatically to the third page.

>8) if the patch is a single file couldn't the wizard try and select the file
>for me for free, it has the path etc... so it should be able to do this, in
>fact would it be true that once a project is selected there is always enough
>information to make the right selection?

The problem is that there's no way to guarantee that the patch has been
generated with full project path info. It might have been generated with a
relative path in which case, there would be no way to efficiently determine
where to apply the patch. This second page is intended to work exactly the same
as the old page.


Page 3:
>9) type in the text "d" for "Maximum fuzz factor", no warning or error is
>provided, what is the fuzz factor, how does it work?  When I press Guess what
>happens?

TODO

>11) when I select an item and the source compare opens, there is a lot of
>wasted space below it... see attachment

TODO

>13)sometimes there are little red exclamation icons next to items, but there is
>no information provided as to what this means, and how can I fix it?

TODO (provide Hunk error description)

>14) when making the dialog wider there is no need for the "Reverse Patch" and
>"Ignore white space" checkboxes to move to the right... they should stick to
>the left

TODO
Comment 6 Michael Van Meekeren CLA 2006-04-05 16:54:47 EDT
NOTE: FYI heres the comments from a team who wants to see this same item fixed for 3.2 as a polish item.  I believe it is similar but thought I should paste it here:

Cleanup UI of Create/Apply Patch wizards (follow Eclipse UI guidelines):
button sizes
caption
margins
mnemonics (missing)
polish all descriptions and messages
Match Project... button launches dialog called Retarget Patch (inconsistent)


Comment 7 Bogdan Gheorghe CLA 2006-04-12 15:44:55 EDT
Bug 132275 fixed a bunch of these for the Apply Patch (thanks!) I'm attaching a patch that will take care of the remaining Apply Patch issues (button name change and an enablement fix). 

(As for the Create Patch side of things, I've already committed the fixes.)
Comment 8 Bogdan Gheorghe CLA 2006-04-12 15:48:23 EDT
*** Bug 132693 has been marked as a duplicate of this bug. ***
Comment 9 Bogdan Gheorghe CLA 2006-04-12 16:18:32 EDT
Created attachment 38447 [details]
Apply Patch improvements
Comment 10 Bogdan Gheorghe CLA 2006-04-13 00:11:41 EDT
Created attachment 38484 [details]
Updated Apply Patch patch
Comment 11 Andre Weinand CLA 2006-04-18 09:14:00 EDT
I've verified and applied the patch.
Thanks, Bogdan.
Comment 12 Bogdan Gheorghe CLA 2006-04-19 12:32:19 EDT
Patch applied.
Comment 13 Bogdan Gheorghe CLA 2006-04-27 12:16:20 EDT
Verified