# [egit PATCH] avoiding output in the error log 5 messages from 2008-08-27 to 2008-08-27. Participants: robert_no.spam_m@yahoo.fr, Robin Rosenberg, Shawn O. Pearce. Thread: https://gitlist.dev/t/15215 ## robert_no.spam_m@yahoo.fr, 2008-08-27 09:34 Subject: [egit PATCH] avoiding output in the error log Message-ID: <218798.11648.bm@omp221.mail.ukl.yahoo.com> URL: https://gitlist.dev/e/218798.11648.bm%40omp221.mail.ukl.yahoo.com ``` Hi,this is a patch for egit. The purpose of the patch is to avoid a lot of output in the error log. >From 6f7be5d1a41d356a9b558ac81722e959095ff04d Mon Sep 17 00:00:00 2001 From: robert Date: Mon, 25 Aug 2008 20:42:39 +0200 Subject: [PATCH] Checks added to avoid a lot of entries in the log * IResource.exists() must be checked before calling setProperty on it. * getActiveDecorator() was returning null in some case. --- .../internal/decorators/GitResourceDecorator.java | 10 +++++++--- 1 files changed, 7 insertions(+), 3 deletions(-) diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java index 5857eaf..7c10ec6 100644 --- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java +++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java @@ -143,7 +143,9 @@ if (resources.size() > 0) try { m.accept(new IResourceVisitor() { public boolean visit(IResource resource) throws CoreException { - getActiveDecorator().clearDecorationState(resource); + GitResourceDecorator decorator = getActiveDecorator(); + if (decorator != null) + decorator.clearDecorationState(resource); return true; } }, @@ -197,8 +199,10 @@ synchronized (resources) { } // End ResCL void clearDecorationState(IResource r) throws CoreException { - r.setSessionProperty(GITFOLDERDIRTYSTATEPROPERTY, null); - fireLabelProviderChanged(new LabelProviderChangedEvent(this, r)); + if (r.exists()) { + r.setSessionProperty(GITFOLDERDIRTYSTATEPROPERTY, null); + fireLabelProviderChanged(new LabelProviderChangedEvent(this, r)); + } } static ResCL myrescl = new ResCL(); -- 1.6.0 ``` ## Robin Rosenberg, 2008-08-27 20:28 Subject: Re: [egit PATCH] avoiding output in the error log Message-ID: <200808272228.07060.robin.rosenberg.lists@dewire.com> URL: https://gitlist.dev/e/200808272228.07060.robin.rosenberg.lists%40dewire.com In-Reply-To: <218798.11648.bm@omp221.mail.ukl.yahoo.com> ``` onsdagen den 27 augusti 2008 11.34.33 skrev robert_no.spam_m@yahoo.fr: > Hi,this is a patch for egit. The purpose of the patch > is to avoid a lot of output in the error log. Welcome Robert! > > From 6f7be5d1a41d356a9b558ac81722e959095ff04d Mon Sep 17 00:00:00 2001 > From: robert > Date: Mon, 25 Aug 2008 20:42:39 +0200 > Subject: [PATCH] Checks added to avoid a lot of entries in the log > > * IResource.exists() must be checked before calling setProperty on it. > * getActiveDecorator() was returning null in some case. > --- > .../internal/decorators/GitResourceDecorator.java | 10 +++++++--- > 1 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java > index 5857eaf..7c10ec6 100644 > --- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java > +++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitResourceDecorator.java > @@ -143,7 +143,9 @@ if (resources.size() > 0) > try { > m.accept(new IResourceVisitor() { > public boolean visit(IResource resource) throws CoreException { > - getActiveDecorator().clearDecorationState(resource); > + GitResourceDecorator decorator = getActiveDecorator(); > + if (decorator != null) > + decorator.clearDecorationState(resource); When doest his happen? > return true; > } > }, > @@ -197,8 +199,10 @@ synchronized (resources) { > } // End ResCL > > void clearDecorationState(IResource r) throws CoreException { > - r.setSessionProperty(GITFOLDERDIRTYSTATEPROPERTY, null); > - fireLabelProviderChanged(new LabelProviderChangedEvent(this, r)); > + if (r.exists()) { > + r.setSessionProperty(GITFOLDERDIRTYSTATEPROPERTY, null); > + fireLabelProviderChanged(new LabelProviderChangedEvent(this, r)); > + } Shawn, you removed the test from the code in 4a230ea1. Perhaps you could care to comment on this patch to restore the test, though slightly different. if (r.isAccessible()) r.setSessionProperty(GITFOLDERDIRTYSTATEPROPERTY, null); -- robin ``` ## Shawn O. Pearce, 2008-08-27 20:33 Subject: Re: [egit PATCH] avoiding output in the error log Message-ID: <20080827203343.GN26523@spearce.org> URL: https://gitlist.dev/e/20080827203343.GN26523%40spearce.org In-Reply-To: <200808272228.07060.robin.rosenberg.lists@dewire.com> ``` Robin Rosenberg wrote: > > @@ -197,8 +199,10 @@ synchronized (resources) { > > } // End ResCL > > > > void clearDecorationState(IResource r) throws CoreException { > > - r.setSessionProperty(GITFOLDERDIRTYSTATEPROPERTY, null); > > - fireLabelProviderChanged(new LabelProviderChangedEvent(this, r)); > > + if (r.exists()) { > > + r.setSessionProperty(GITFOLDERDIRTYSTATEPROPERTY, null); > > + fireLabelProviderChanged(new LabelProviderChangedEvent(this, r)); > > + } > > Shawn, you removed the test from the code in 4a230ea1. Perhaps you could care to > comment on this patch to restore the test, though slightly different. > > if (r.isAccessible()) > r.setSessionProperty(GITFOLDERDIRTYSTATEPROPERTY, null); This may be a patch queue reordering bug. If you look at 4a230ea1 I moved the isAccessible test higher up, such that we shouldn't enter into clearDecorationState unless the resource is accessible. But there may currently be two code paths that enter this method, and a patch I did not yet submit had removed the other code path. So in the short/medium term I agree with your patch, basically undo the second hunk of 4a230ea1. But keep the middle hunk, it was a real bug I ran into. -- Shawn. ``` ## Robin Rosenberg, 2008-08-27 21:50 Subject: Re: [egit PATCH] avoiding output in the error log Message-ID: <200808272350.56819.robin.rosenberg.lists@dewire.com> URL: https://gitlist.dev/e/200808272350.56819.robin.rosenberg.lists%40dewire.com In-Reply-To: <20080827203343.GN26523@spearce.org> ``` onsdagen den 27 augusti 2008 22.33.43 skrev Shawn O. Pearce: > Robin Rosenberg wrote: > > > @@ -197,8 +199,10 @@ synchronized (resources) { > > > } // End ResCL > > > > > > void clearDecorationState(IResource r) throws CoreException { > > > - r.setSessionProperty(GITFOLDERDIRTYSTATEPROPERTY, null); > > > - fireLabelProviderChanged(new LabelProviderChangedEvent(this, r)); > > > + if (r.exists()) { > > > + r.setSessionProperty(GITFOLDERDIRTYSTATEPROPERTY, null); > > > + fireLabelProviderChanged(new LabelProviderChangedEvent(this, r)); > > > + } > > > > Shawn, you removed the test from the code in 4a230ea1. Perhaps you could care to > > comment on this patch to restore the test, though slightly different. > > > > if (r.isAccessible()) > > r.setSessionProperty(GITFOLDERDIRTYSTATEPROPERTY, null); > > This may be a patch queue reordering bug. > > If you look at 4a230ea1 I moved the isAccessible test higher up, > such that we shouldn't enter into clearDecorationState unless the > resource is accessible. But there may currently be two code paths > that enter this method, and a patch I did not yet submit had removed > the other code path. Good. So then the question is whether we should use isAccessible() or exists()? Normally this makes no difference, but for a Project (at least) there is a difference. I project that exits is not accessible when closed. -- robin ``` ## Shawn O. Pearce, 2008-08-27 22:00 Subject: Re: [egit PATCH] avoiding output in the error log Message-ID: <20080827220022.GP26523@spearce.org> URL: https://gitlist.dev/e/20080827220022.GP26523%40spearce.org In-Reply-To: <200808272350.56819.robin.rosenberg.lists@dewire.com> ``` Robin Rosenberg wrote: > > > > > > Shawn, you removed the test from the code in 4a230ea1. Perhaps you could care to > > > comment on this patch to restore the test, though slightly different. > > > > > > if (r.isAccessible()) > > > r.setSessionProperty(GITFOLDERDIRTYSTATEPROPERTY, null); > > Good. So then the question is whether we should use isAccessible() or exists()? Normally > this makes no difference, but for a Project (at least) there is a difference. I project that exits > is not accessible when closed. It _has_ to be isAccessible(). You cannot get or set a session property on a closed project. -- Shawn. ```