Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 166837

Summary:	Wizard from ApplyPatchOperation does not skip the patch selection page.
Product:	[Eclipse Project] Platform	Reporter:	Eugene Kuleshov <ekuleshov>
Component:	Team	Assignee:	Michael Valenta <Michael.Valenta>
Status:	VERIFIED FIXED	QA Contact:
Severity:	critical
Priority:	P3	CC:	mik.kersten
Version:	3.3
Target Milestone:	3.3 M4
Hardware:	PC
OS:	Windows XP
Whiteboard:
Bug Depends on:
Bug Blocks:	103932

Description Eugene Kuleshov

2006-12-05 15:08:05 EST

The JavaDoc for ApplyPatchOperation constructor promise the following:

- If a patch
is supplied, the initial input page is skipped. If a patch is not
supplied the wizard
will open on the input page.

- If the patch is a workspace patch, the target selection
page is skipped and
the preview page is displayed.

- If the patch is not a workspace
patch and the target is specified, then the
target page is skipped and the preview page
is displayed. If a target is not
specified, the wizard will open on the target selection
page.

But I can't find this login neither in ApplyPatchOperation, nor in PatchWizard
or
its pages.

I am running on 3.3M3 and been trying to call ApplyPatchOperation. It always
show
the page that require to choose patch source (clipboard, file, workspace).
It doesn't
look like method PatchWizard.getNextPage() is not called when wizard
is shown, and first
page is not skipped and as a result it not possible to pass 
the patch content to ApplyPatchOperation.

PS:
Also, can you please use ISource.getContents() to read patch content instead of
reading
it from the file?

Comment 1 Michael Valenta

2006-12-05 15:26:35 EST

What is ISource? Do you mean IStorage? I'm also not sure what you mean by "But I can't find this login neither in ApplyPatchOperation, nor in PatchWizard or its pages".

Anyway, I should be able to find the time to look at this this week so a fix can get into 3.3 M4.

Comment 2 Eugene Kuleshov

2006-12-05 15:37:13 EST

(In reply to comment #1)
> What is ISource? Do you mean IStorage? 

Yes, sorry. It should be IStorage.getContent()

> I'm also not sure what you mean by "But I can't find this login 
> neither in ApplyPatchOperation, nor in PatchWizard or its pages".

Another typo. Please read that as "...can't find this logic..."

> Anyway, I should be able to find the time to look at this this week so a fix
> can get into 3.3 M4.

Thanks. I hoped to make this for Mylar 1.0, but it doesn't look very realistic now...

Comment 3 Mik Kersten

2006-12-05 18:33:59 EST

I gave this a try a while back and came up against the problems that Eugene has summarized, apologies that I did not comment but I did not have time to verify whether there was a work-around and thankfully Eugene picked this up.

Eugene: his has to wait post 1.0 since we froze new features with 0.9.1, but I do realize that it would be helpful for a lot of people.

Comment 4 Michael Valenta

2006-12-06 11:51:07 EST

Fix released to HEAD. The patch selection page is now skipped if a patch is provide and IStorage#getContents() is used. The javadoc was changed to indicate that the target page is displayed even if a target is provided as this has always been the behaviour of the wizard.

Comment 5 Mik Kersten

2006-12-06 13:01:27 EST

Excellent!  

Eugene: unless you indicate otherwise I assume you'll give this a whirl on bug 103932, and hopefully we can get it out when we do our 3.3M4 based release.

Comment 6 Michael Valenta

2006-12-12 09:59:31 EST

Verified

Comment 7 Mik Kersten

2006-12-21 13:51:41 EST

Verified to support Mylar bug 103932, which involved putting patch contents from a bug report into an IStorage in the following way:

IStorage storage = new IStorage() {
		public InputStream getContents() throws CoreException {
			return new StringBufferInputStream(contents);
		}
...
}

Comment 8 Eugene Kuleshov

2006-12-21 14:39:22 EST

By the way, Michael, current PatchWizard is reading full path from IStorage that contains patch. See line 123:

  fPatcher.setName(patch.getFullPath().toString());
  
It forced us to provide some bogus full path (else getting a NPE exception if patch.getFullPath() returns null), because we don't have any file laying around with patch content. I looked around and that full path value doesn't seem used after that. So, you probably should remove that call to getFullPath()

Comment 9 Michael Valenta

2006-12-22 08:14:25 EST

Could you log a separate bug report for that please?

Comment 10 Eugene Kuleshov

2006-12-22 14:09:55 EST

 (In reply to comment #9)
> Could you log a separate bug report for that please?

Done. Bug 168947