subclipse svn commit: r5091 - trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java

[Mark Phippard]

Drive by review ...

Is this the right thing to do? Doesn't SVN delete the actual folder/file?
Is this change only applying to some specific scenario?

Author: selsemore
Date: 2011-08-22 06:33:24-0700
New Revision: 5091
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java
Move/delete hook should return false for deleteFile and deleteFolder
methods
(so that Eclipse will do its thing and do the actual delete after Subclipse
is done).
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java
http://subclipse.tigris.org/source/browse/subclipse/trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java?view=diff&pathrev=5091&r1=5090&r2=5091
==============================================================================
---
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java
(original)
+++
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java
2011-08-22 06:33:24-0700
@@ -74,15 +74,12 @@
deletedFiles.add(file);
deleteResource(resource);
-
- tree.deletedFile(file);
-
} catch (SVNException e) {
tree.failed(e.getStatus());
} finally {
monitor.done();
}
- return true;
+ return false;
}
@@ -113,8 +110,7 @@
} finally {
monitor.done();
}
- return true;
-
+ return false;
}
/*
------------------------------------------------------
http://subclipse.tigris.org/ds/viewMessage.do?dsForumId=3541&dsMessageId=2827597
To unsubscribe from this discussion, please e-mail [unsubscribeURL]

--
Thanks

Mark Phippard
http://markphip.blogspot.com/

------------------------------------------------------
http://subclipse.tigris.org/ds/viewMessage.do?dsForumId=1043&dsMessageId=2827599

To unsubscribe from this discussion, e-mail: [dev-***@subclipse.tigris.org].

[Stephen Elsemore]

Eclipse doesn't have a problem if the resource has actually already been
deleted. But I suppose this approach could cause a problem if there is
some other move/delete hook in place that will get an error if it tries
to handle the delete and the resource is not there. The alternative
solution is to make sure our move/delete hook replicates everything that
the Eclipse delete does.



From: Mark Phippard [mailto:***@gmail.com]
Sent: Monday, August 22, 2011 6:36 AM
To: ***@subclipse.tigris.org
Subject: [Subclipse-dev] Re: subclipse svn commit: r5091 -
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subv
ersion/subclipse/core/resources/SVNMoveDeleteHook.java



Drive by review ...



Is this the right thing to do? Doesn't SVN delete the actual
folder/file? Is this change only applying to some specific scenario?





On Mon, Aug 22, 2011 at 9:33 AM, <***@tigris.org> wrote:

Author: selsemore
Date: 2011-08-22 06:33:24-0700
New Revision: 5091

Modified:

trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subv
ersion/subclipse/core/resources/SVNMoveDeleteHook.java

Log:
Move/delete hook should return false for deleteFile and deleteFolder
methods
(so that Eclipse will do its thing and do the actual delete after
Subclipse
is done).

Modified:
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subv
ersion/subclipse/core/resources/SVNMoveDeleteHook.java
Url:
http://subclipse.tigris.org/source/browse/subclipse/trunk/subclipse/org.
tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/cor
e/resources/SVNMoveDeleteHook.java?view=diff&pathrev=5091&r1=5090&r2=509
1
========================================================================
======
---
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subv
ersion/subclipse/core/resources/SVNMoveDeleteHook.java (original)
+++
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subv
ersion/subclipse/core/resources/SVNMoveDeleteHook.java 2011-08-22
06:33:24-0700
@@ -74,15 +74,12 @@
deletedFiles.add(file);

deleteResource(resource);
-
- tree.deletedFile(file);
-
} catch (SVNException e) {
tree.failed(e.getStatus());
} finally {
monitor.done();
}
- return true;
+ return false;

}

@@ -113,8 +110,7 @@
} finally {
monitor.done();
}
- return true;
-
+ return false;
}

/*

------------------------------------------------------
http://subclipse.tigris.org/ds/viewMessage.do?dsForumId=3541&dsMessageId
=2827597

To unsubscribe from this discussion, please e-mail [unsubscribeURL]
--
Thanks

Mark Phippard
http://markphip.blogspot.com/

------------------------------------------------------
http://subclipse.tigris.org/ds/viewMessage.do?dsForumId=1043&dsMessageId=2827602

To unsubscribe from this discussion, e-mail: [dev-***@subclipse.tigris.org].

[Mark Phippard]

Reading between the lines, it sounds like you are saying you are doing this
so that Eclipse will trigger the proper refreshes after the operation
completes?

Did you look at why our NotificationHandler would not simply do it when it
receives the Delete notifications?

Eclipse doesn’t have a problem if the resource has actually already been
deleted. But I suppose this approach could cause a problem if there is some
other move/delete hook in place that will get an error if it tries to handle
the delete and the resource is not there. The alternative solution is to
make sure our move/delete hook replicates everything that the Eclipse delete
does.****
** **
*Sent:* Monday, August 22, 2011 6:36 AM
*Subject:* [Subclipse-dev] Re: subclipse svn commit: r5091 -
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java
****
** **
Drive by review ...****
** **
Is this the right thing to do? Doesn't SVN delete the actual folder/file?
Is this change only applying to some specific scenario?****
** **
** **
Author: selsemore
Date: 2011-08-22 06:33:24-0700
New Revision: 5091
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java
Move/delete hook should return false for deleteFile and deleteFolder
methods
(so that Eclipse will do its thing and do the actual delete after Subclipse
is done).
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java
http://subclipse.tigris.org/source/browse/subclipse/trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java?view=diff&pathrev=5091&r1=5090&r2=5091
==============================================================================
---
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java
(original)
+++
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java
2011-08-22 06:33:24-0700
@@ -74,15 +74,12 @@
deletedFiles.add(file);
deleteResource(resource);
-
- tree.deletedFile(file);
-
} catch (SVNException e) {
tree.failed(e.getStatus());
} finally {
monitor.done();
}
- return true;
+ return false;
}
@@ -113,8 +110,7 @@
} finally {
monitor.done();
}
- return true;
-
+ return false;
}
/*
------------------------------------------------------
http://subclipse.tigris.org/ds/viewMessage.do?dsForumId=3541&dsMessageId=2827597
To unsubscribe from this discussion, please e-mail [unsubscribeURL]****
****
** **
--
Thanks
Mark Phippard
http://markphip.blogspot.com/****

--
Thanks

Mark Phippard
http://markphip.blogspot.com/

------------------------------------------------------
http://subclipse.tigris.org/ds/viewMessage.do?dsForumId=1043&dsMessageId=2827605

To unsubscribe from this discussion, e-mail: [dev-***@subclipse.tigris.org].

[Stephen Elsemore]

My guess would be that we didn’t receive the Delete notifications because our move/delete hook did not trigger them, and the Eclipse delete (which would have triggered them) did not happen. I’ll look into doing everything in our move/delete hook.



From: Mark Phippard [mailto:***@gmail.com]
Sent: Monday, August 22, 2011 6:47 AM
To: ***@subclipse.tigris.org
Subject: Re: [Subclipse-dev] Re: subclipse svn commit: r5091 - trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java



Reading between the lines, it sounds like you are saying you are doing this so that Eclipse will trigger the proper refreshes after the operation completes?



Did you look at why our NotificationHandler would not simply do it when it receives the Delete notifications?





On Mon, Aug 22, 2011 at 9:45 AM, Stephen Elsemore <***@collab.net> wrote:

Eclipse doesnt have a problem if the resource has actually already been deleted. But I suppose this approach could cause a problem if there is some other move/delete hook in place that will get an error if it tries to handle the delete and the resource is not there. The alternative solution is to make sure our move/delete hook replicates everything that the Eclipse delete does.

From: Mark Phippard [mailto:***@gmail.com]
Sent: Monday, August 22, 2011 6:36 AM
To: ***@subclipse.tigris.org
Subject: [Subclipse-dev] Re: subclipse svn commit: r5091 - trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java

Drive by review ...

Is this the right thing to do? Doesn't SVN delete the actual folder/file? Is this change only applying to some specific scenario?

On Mon, Aug 22, 2011 at 9:33 AM, <***@tigris.org> wrote:

Author: selsemore
Date: 2011-08-22 06:33:24-0700
New Revision: 5091

Modified:
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java

Log:
Move/delete hook should return false for deleteFile and deleteFolder methods
(so that Eclipse will do its thing and do the actual delete after Subclipse
is done).

Modified: trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java
Url: http://subclipse.tigris.org/source/browse/subclipse/trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java?view=diff&pathrev=5091&r1=5090&r2=5091
==============================================================================
--- trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java (original)
+++ trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java 2011-08-22 06:33:24-0700
@@ -74,15 +74,12 @@
deletedFiles.add(file);

deleteResource(resource);
-
- tree.deletedFile(file);
-
} catch (SVNException e) {
tree.failed(e.getStatus());
} finally {
monitor.done();
}
- return true;
+ return false;

}

@@ -113,8 +110,7 @@
} finally {
monitor.done();
}
- return true;
-
+ return false;
}

/*

------------------------------------------------------
http://subclipse.tigris.org/ds/viewMessage.do?dsForumId=3541&dsMessageId=2827597

To unsubscribe from this discussion, please e-mail [unsubscribeURL]
--
Thanks

Mark Phippard
http://markphip.blogspot.com/
--
Thanks

Mark Phippard
http://markphip.blogspot.com/

------------------------------------------------------
http://subclipse.tigris.org/ds/viewMessage.do?dsForumId=1043&dsMessageId=2827606

To unsubscribe from this discussion, e-mail: [dev-***@subclipse.tigris.org].

[Mark Phippard]

The console showed SVN messages about delete, so that means it went to our
NotificationHandler. Do we sent notifications to refresh resources when we
receive the delete notification from SVN?

Post by Stephen Elsemore
My guess would be that we didn’t receive the Delete notifications because
our move/delete hook did not trigger them, and the Eclipse delete (which
would have triggered them) did not happen. I’ll look into doing everything
in our move/delete hook.****
** **
*Sent:* Monday, August 22, 2011 6:47 AM
*Subject:* Re: [Subclipse-dev] Re: subclipse svn commit: r5091 -
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java
****
** **
Reading between the lines, it sounds like you are saying you are doing this
so that Eclipse will trigger the proper refreshes after the operation
completes?****
** **
Did you look at why our NotificationHandler would not simply do it when it
receives the Delete notifications?****
** **
** **
wrote:****
Eclipse doesnt have a problem if the resource has actually already been
deleted. But I suppose this approach could cause a problem if there is some
other move/delete hook in place that will get an error if it tries to handle
the delete and the resource is not there. The alternative solution is to
make sure our move/delete hook replicates everything that the Eclipse delete
does.****
*Sent:* Monday, August 22, 2011 6:36 AM
*Subject:* [Subclipse-dev] Re: subclipse svn commit: r5091 -
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java
****
Drive by review ...****
Is this the right thing to do? Doesn't SVN delete the actual folder/file?
Is this change only applying to some specific scenario?****
Author: selsemore
Date: 2011-08-22 06:33:24-0700
New Revision: 5091
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java
Move/delete hook should return false for deleteFile and deleteFolder methods
(so that Eclipse will do its thing and do the actual delete after Subclipse
is done).
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java
http://subclipse.tigris.org/source/browse/subclipse/trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java?view=diff&pathrev=5091&r1=5090&r2=5091
==============================================================================
---
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java
(original)
+++
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java
2011-08-22 06:33:24-0700
@@ -74,15 +74,12 @@
deletedFiles.add(file);
deleteResource(resource);
-
- tree.deletedFile(file);
-
} catch (SVNException e) {
tree.failed(e.getStatus());
} finally {
monitor.done();
}
- return true;
+ return false;
}
@@ -113,8 +110,7 @@
} finally {
monitor.done();
}
- return true;
-
+ return false;
}
/*
------------------------------------------------------
http://subclipse.tigris.org/ds/viewMessage.do?dsForumId=3541&dsMessageId=2827597
To unsubscribe from this discussion, please e-mail [unsubscribeURL]****
****
--
Thanks
Mark Phippard
http://markphip.blogspot.com/****
****
** **
--
Thanks
Mark Phippard
http://markphip.blogspot.com/****

--
Thanks

Mark Phippard
http://markphip.blogspot.com/

------------------------------------------------------
http://subclipse.tigris.org/ds/viewMessage.do?dsForumId=1043&dsMessageId=2827607

To unsubscribe from this discussion, e-mail: [dev-***@subclipse.tigris.org].

[Stephen Elsemore]

We shouldn’t because we expect our resource change listener to handle it. Which it will, as long as our move/delete hook does what it needs to do to trigger it, or delegates it to Eclipse.



From: Mark Phippard [mailto:***@gmail.com]
Sent: Monday, August 22, 2011 6:54 AM
To: ***@subclipse.tigris.org
Subject: Re: [Subclipse-dev] Re: subclipse svn commit: r5091 - trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java



The console showed SVN messages about delete, so that means it went to our NotificationHandler. Do we sent notifications to refresh resources when we receive the delete notification from SVN?





On Mon, Aug 22, 2011 at 9:50 AM, Stephen Elsemore <***@collab.net> wrote:

My guess would be that we didnt receive the Delete notifications because our move/delete hook did not trigger them, and the Eclipse delete (which would have triggered them) did not happen. Ill look into doing everything in our move/delete hook.

From: Mark Phippard [mailto:***@gmail.com]
Sent: Monday, August 22, 2011 6:47 AM


To: ***@subclipse.tigris.org

Subject: Re: [Subclipse-dev] Re: subclipse svn commit: r5091 - trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java

Reading between the lines, it sounds like you are saying you are doing this so that Eclipse will trigger the proper refreshes after the operation completes?

Did you look at why our NotificationHandler would not simply do it when it receives the Delete notifications?

On Mon, Aug 22, 2011 at 9:45 AM, Stephen Elsemore <***@collab.net> wrote:

Eclipse doesnt have a problem if the resource has actually already been deleted. But I suppose this approach could cause a problem if there is some other move/delete hook in place that will get an error if it tries to handle the delete and the resource is not there. The alternative solution is to make sure our move/delete hook replicates everything that the Eclipse delete does.

From: Mark Phippard [mailto:***@gmail.com]
Sent: Monday, August 22, 2011 6:36 AM
To: ***@subclipse.tigris.org
Subject: [Subclipse-dev] Re: subclipse svn commit: r5091 - trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java

Drive by review ...

Is this the right thing to do? Doesn't SVN delete the actual folder/file? Is this change only applying to some specific scenario?

On Mon, Aug 22, 2011 at 9:33 AM, <***@tigris.org> wrote:

Author: selsemore
Date: 2011-08-22 06:33:24-0700
New Revision: 5091

Modified:
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java

Log:
Move/delete hook should return false for deleteFile and deleteFolder methods
(so that Eclipse will do its thing and do the actual delete after Subclipse
is done).

Modified: trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java
Url: http://subclipse.tigris.org/source/browse/subclipse/trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java?view=diff&pathrev=5091&r1=5090&r2=5091
==============================================================================
--- trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java (original)
+++ trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java 2011-08-22 06:33:24-0700
@@ -74,15 +74,12 @@
deletedFiles.add(file);

deleteResource(resource);
-
- tree.deletedFile(file);
-
} catch (SVNException e) {
tree.failed(e.getStatus());
} finally {
monitor.done();
}
- return true;
+ return false;

}

@@ -113,8 +110,7 @@
} finally {
monitor.done();
}
- return true;
-
+ return false;
}

/*

------------------------------------------------------
http://subclipse.tigris.org/ds/viewMessage.do?dsForumId=3541&dsMessageId=2827597

To unsubscribe from this discussion, please e-mail [unsubscribeURL]
--
Thanks

Mark Phippard
http://markphip.blogspot.com/
--
Thanks

Mark Phippard
http://markphip.blogspot.com/
--
Thanks

Mark Phippard
http://markphip.blogspot.com/

------------------------------------------------------
http://subclipse.tigris.org/ds/viewMessage.do?dsForumId=1043&dsMessageId=2827608

To unsubscribe from this discussion, e-mail: [dev-***@subclipse.tigris.org].

[jcompagner]

so what happens exactly if i delete a resource from outside of eclipse
and then press refresh?
What kind of events do you get?

Eclipse doesn’t have a problem if the resource has actually already been
deleted.  But I suppose this approach could cause a problem if there is some
other move/delete hook in place that will get an error if it tries to handle
the delete and the resource is not there.  The alternative solution is to
make sure our move/delete hook replicates everything that the Eclipse delete
does.
Sent: Monday, August 22, 2011 6:36 AM
Subject: [Subclipse-dev] Re: subclipse svn commit: r5091 -
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java
Drive by review ...
Is this the right thing to do?  Doesn't SVN delete the actual folder/file?
 Is this change only applying to some specific scenario?
Author: selsemore
Date: 2011-08-22 06:33:24-0700
New Revision: 5091
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java
Move/delete hook should return false for deleteFile and deleteFolder methods
(so that Eclipse will do its thing and do the actual delete after Subclipse
is done).
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java
http://subclipse.tigris.org/source/browse/subclipse/trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java?view=diff&pathrev=5091&r1=5090&r2=5091
==============================================================================
---
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java
     (original)
+++
trunk/subclipse/org.tigris.subversion.subclipse.core/src/org/tigris/subversion/subclipse/core/resources/SVNMoveDeleteHook.java
     2011-08-22 06:33:24-0700
@@ -74,15 +74,12 @@
            deletedFiles.add(file);
            deleteResource(resource);
-
-            tree.deletedFile(file);
-
        } catch (SVNException e) {
            tree.failed(e.getStatus());
        } finally {
            monitor.done();
        }
-        return true;
+        return false;
    }
@@ -113,8 +110,7 @@
        } finally {
            monitor.done();
        }
-        return true;
-
+        return false;
    }
    /*
------------------------------------------------------
http://subclipse.tigris.org/ds/viewMessage.do?dsForumId=3541&dsMessageId=2827597
To unsubscribe from this discussion, please e-mail [unsubscribeURL]
--
Thanks
Mark Phippard
http://markphip.blogspot.com/

------------------------------------------------------
http://subclipse.tigris.org/ds/viewMessage.do?dsForumId=1043&dsMessageId=2827633

To unsubscribe from this discussion, e-mail: [dev-***@subclipse.tigris.org].