[egit-jgit] excluded patterns are decorated as being untracked

[egit-jgit] excluded patterns are decorated as being untracked

[Galder Zamarreno]

Hi,

I've been using egit for a few weeks and firstly, I'd like to thank the 
people involved in the project for the work done so far.

There's one thing that has been bugging me about egit though which is 
related to the decoration of untracked files. Egit/JGit does not seem to 
pay attention to .git/info/exclude that I have configured so that 
anything under output folder is excluded.

Egit/JGit does seem to have decorations working fine for patterns 
specified in "Team/Ignored Resources" but it only applies it to files 
not folders, hence, adding "output" as pattern does not work and 
instead, I have to specify any pattern that would match a file within 
the output folder which is not practical. Folders are taken into account 
as ignored resources in subeclipse (subversion eclipse plugin)

I can see two ways of implementing this that I'm happy to have a look 
into but I wanted to get some advice from the experts of egit/jgit to 
indicate which would be the preferred solution going forward.

1.- Implement .git/info/exclude functionality in egit/jgit

2.- Improve the decoration handling in jgit/egit so that it can check 
whether the file is under a pattern that should be excluded. I tried to 
implement this but requires using API that eclipse considers internal.

What do people think? Should I bother with 2 or is it better to 
implement decorations for patterns in .git/info/exclude correctly?

Thanks!
-- 
Galder Zamarreño
Sr. Software Maintenance Engineer
JBoss, a division of Red Hat

Re: [egit-jgit] excluded patterns are decorated as being untracked

[Robin Rosenberg]

tisdagen den 17 juni 2008 18.43.12 skrev Galder Zamarreno:
> Hi,
> 
> I've been using egit for a few weeks and firstly, I'd like to thank the 
> people involved in the project for the work done so far.
> 
> There's one thing that has been bugging me about egit though which is 
> related to the decoration of untracked files. Egit/JGit does not seem to 
> pay attention to .git/info/exclude that I have configured so that 
> anything under output folder is excluded.
This is a correct observation. It is a missing feature so far.

> Egit/JGit does seem to have decorations working fine for patterns 
> specified in "Team/Ignored Resources" but it only applies it to files 
> not folders, hence, adding "output" as pattern does not work and 
I haven't noticed, but looking at the code its seems some we should
use Team.isIgnoredHint instead of what we do now (which only
take a file as an argument). Patch below. The reason I haven't noticed
is that I have only depended on Eclipse marking of resources as "derived"
which worked. (You can mark resources as derived youself in the properties
of that resource, and Egit will ignore it).

> instead, I have to specify any pattern that would match a file within 
> the output folder which is not practical. Folders are taken into account 
> as ignored resources in subeclipse (subversion eclipse plugin)
> 
> I can see two ways of implementing this that I'm happy to have a look 
> into but I wanted to get some advice from the experts of egit/jgit to 
> indicate which would be the preferred solution going forward.
> 
> 1.- Implement .git/info/exclude functionality in egit/jgit
We need that. Florian came up with a set of patches that should be
usable for this. I haven't applied them to the tree yet.

> 2.- Improve the decoration handling in jgit/egit so that it can check 
> whether the file is under a pattern that should be excluded. I tried to 
> implement this but requires using API that eclipse considers internal.
> 
> What do people think? Should I bother with 2 or is it better to 
> implement decorations for patterns in .git/info/exclude correctly?

The decoration should not ignore anything except what the resource filters
(affecting visibility) dictates. If a resource is tracked it should be shown unless
visibility is prevented by a resource filter or other view specific mechanism.
When we track (add) resources recursively we should honor the git ignore
patterns, but only for add. Everywhere else we do not just ignore resources
that match an ignore pattern.

Btw, Here is an attempt to match folders by Team ignore patterns too.

(OT: I did not format the code. (please try, and you'll see why). We'll have
to come up with something better than an 80-column format, it's so seventies)

-- robin

>From 42eadb50b9b87e5324f941ce2d2371e07577f55a Mon Sep 17 00:00:00 2001
From: Robin Rosenberg <robin.rosenberg@dewire.com>
Date: Tue, 17 Jun 2008 22:48:52 +0200
Subject: [PATCH] Apply Team/Ignore settings to folders too when tracking new resources

We used to only care for file resources. Now folders, and their content,
are ignored if the folder name matches a pattern marked as ignored
in the Team settings.


Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../org/spearce/egit/core/op/TrackOperation.java   |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/org.spearce.egit.core/src/org/spearce/egit/core/op/TrackOperation.java b/org.spearce.egit.core/src/org/spearce/egit/core/op/TrackOperation.java
index 6521f1c..af16cdb 100644
--- a/org.spearce.egit.core/src/org/spearce/egit/core/op/TrackOperation.java
+++ b/org.spearce.egit.core/src/org/spearce/egit/core/op/TrackOperation.java
@@ -90,13 +90,20 @@ public class TrackOperation implements IWorkspaceRunnable {
 							public boolean visit(IResource resource) throws CoreException {
 								try {
 									String repoPath = rm.getRepoRelativePath(resource);
+									// We use add to reset the assume valid bit, so we check the bit
+									// first. If a resource within a ignored folder is marked
+									// we ignore it here, i.e. there is no way to unmark it expect
+									// by explicitly selecting and invoking track on it.
 									if (resource.getType() == IResource.FILE) {
 										Entry entry = index.getEntry(repoPath);
-										if (!Team.isIgnored((IFile)resource) || entry != null && entry.isAssumedValid()) {
+										if (!Team.isIgnoredHint(resource) || entry != null && entry.isAssumedValid()) {
 											entry = index.add(rm.getWorkDir(), new File(rm.getWorkDir(), repoPath));
 											entry.setAssumeValid(false);
 										}
 									}
+									if (Team.isIgnoredHint(resource))
+										return false;
+
 								} catch (IOException e) {
 									e.printStackTrace();
 									throw Activator.error(CoreText.AddOperation_failed, e);
-- 
1.5.5.1.178.g1f811

Re: [egit-jgit] excluded patterns are decorated as being untracked

[Shawn O. Pearce]

Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
> Btw, Here is an attempt to match folders by Team ignore patterns too.
> 
> (OT: I did not format the code. (please try, and you'll see why). We'll have
> to come up with something better than an 80-column format, it's so seventies)

Ignoring the patch for a second, I'm a die-hard 80 column format
person.  You can pry my 80 column wide terminals from me long after
I'm dead.

In Java it may seem like an 80 column limit is a problem, because
the code often gets nested deep and far to the right, such as in
this example here.  I find that just like in C, when code is this
far indented to the right in Java its too damn complex as-is and
should be refactored into smaller methods, and anonymous types
should perhaps be converted to named inner classes or top-level
classes of their own right.

My laptop display panel is only 1440 pixels wide.  By the time I
get my Eclipse workbench open I have about 90 characters wide for
the editor page, given how I place the other views and non-Eclipse
applications around the edges.  Pushing much beyond 80 characters
wide makes it very hard to read code quickly.

And no, I do not want to tote a laptop with a display that is 9000
pixels wide, thanks.

So IMHO, code in jgit+egit should try to target 80 characters wide
as much as possible.  If you can't hit that you need to rethink how
you have the code structured.  I've never refactored something into
smaller more modular methods (to hit the 80 target) and said "gee,
that refactoring made my code harder to understand, what with those
highly descriptive method names and javadoc I gave everything".

OK, now you can call me crazy.
 
> diff --git a/org.spearce.egit.core/src/org/spearce/egit/core/op/TrackOperation.java b/org.spearce.egit.core/src/org/spearce/egit/core/op/TrackOperation.java
> index 6521f1c..af16cdb 100644
> --- a/org.spearce.egit.core/src/org/spearce/egit/core/op/TrackOperation.java
> +++ b/org.spearce.egit.core/src/org/spearce/egit/core/op/TrackOperation.java
> @@ -90,13 +90,20 @@ public class TrackOperation implements IWorkspaceRunnable {
>  							public boolean visit(IResource resource) throws CoreException {
>  								try {
>  									String repoPath = rm.getRepoRelativePath(resource);
> +									// We use add to reset the assume valid bit, so we check the bit
> +									// first. If a resource within a ignored folder is marked
> +									// we ignore it here, i.e. there is no way to unmark it expect
> +									// by explicitly selecting and invoking track on it.
>  									if (resource.getType() == IResource.FILE) {
>  										Entry entry = index.getEntry(repoPath);
> -										if (!Team.isIgnored((IFile)resource) || entry != null && entry.isAssumedValid()) {
> +										if (!Team.isIgnoredHint(resource) || entry != null && entry.isAssumedValid()) {
>  											entry = index.add(rm.getWorkDir(), new File(rm.getWorkDir(), repoPath));
>  											entry.setAssumeValid(false);
>  										}
>  									}
> +									if (Team.isIgnoredHint(resource))
> +										return false;
> +
>  								} catch (IOException e) {
>  									e.printStackTrace();
>  									throw Activator.error(CoreText.AddOperation_failed, e);

-- 
Shawn.

Re: [egit-jgit] excluded patterns are decorated as being untracked

[Galder Zamarreno]

Hi,

Firstly, thanks to all for the quick response.

Robin Rosenberg wrote:
> tisdagen den 17 juni 2008 18.43.12 skrev Galder Zamarreno:
>> Hi,
>>
>> I've been using egit for a few weeks and firstly, I'd like to thank the 
>> people involved in the project for the work done so far.
>>
>> There's one thing that has been bugging me about egit though which is 
>> related to the decoration of untracked files. Egit/JGit does not seem to 
>> pay attention to .git/info/exclude that I have configured so that 
>> anything under output folder is excluded.
> This is a correct observation. It is a missing feature so far.
> 
>> Egit/JGit does seem to have decorations working fine for patterns 
>> specified in "Team/Ignored Resources" but it only applies it to files 
>> not folders, hence, adding "output" as pattern does not work and 
> I haven't noticed, but looking at the code its seems some we should
> use Team.isIgnoredHint instead of what we do now (which only
> take a file as an argument). Patch below. The reason I haven't noticed
> is that I have only depended on Eclipse marking of resources as "derived"
> which worked. (You can mark resources as derived youself in the properties
> of that resource, and Egit will ignore it).

Hmmmm, is marking a resource as "derived" recursive? i.e. if I mark 
"output" folder as derived, would anything under it be considered 
derived? It'd be a pain to go a mark as derived each and every class.

I suppose you still need the patch to use Team.isIgnoredHint to get 
derived resources to be ignored, correct?

> 
>> instead, I have to specify any pattern that would match a file within 
>> the output folder which is not practical. Folders are taken into account 
>> as ignored resources in subeclipse (subversion eclipse plugin)
>>
>> I can see two ways of implementing this that I'm happy to have a look 
>> into but I wanted to get some advice from the experts of egit/jgit to 
>> indicate which would be the preferred solution going forward.
>>
>> 1.- Implement .git/info/exclude functionality in egit/jgit
> We need that. Florian came up with a set of patches that should be
> usable for this. I haven't applied them to the tree yet.

That'd probably be the proper solution to the decoration issue at hand.

> 
>> 2.- Improve the decoration handling in jgit/egit so that it can check 
>> whether the file is under a pattern that should be excluded. I tried to 
>> implement this but requires using API that eclipse considers internal.
>>
>> What do people think? Should I bother with 2 or is it better to 
>> implement decorations for patterns in .git/info/exclude correctly?
> 
> The decoration should not ignore anything except what the resource filters
> (affecting visibility) dictates. If a resource is tracked it should be shown unless
> visibility is prevented by a resource filter or other view specific mechanism.
> When we track (add) resources recursively we should honor the git ignore
> patterns, but only for add. Everywhere else we do not just ignore resources
> that match an ignore pattern.
> 
> Btw, Here is an attempt to match folders by Team ignore patterns too.

Hmmmm, so the proper way is either marking resources as derived and use 
your patch or implementing .git/info/exclude, correct?

> 
> (OT: I did not format the code. (please try, and you'll see why). We'll have
> to come up with something better than an 80-column format, it's so seventies)
> 
> -- robin
> 
> From 42eadb50b9b87e5324f941ce2d2371e07577f55a Mon Sep 17 00:00:00 2001
> From: Robin Rosenberg <robin.rosenberg@dewire.com>
> Date: Tue, 17 Jun 2008 22:48:52 +0200
> Subject: [PATCH] Apply Team/Ignore settings to folders too when tracking new resources
> 
> We used to only care for file resources. Now folders, and their content,
> are ignored if the folder name matches a pattern marked as ignored
> in the Team settings.
> 
> 
> Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
> ---
>  .../org/spearce/egit/core/op/TrackOperation.java   |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/org.spearce.egit.core/src/org/spearce/egit/core/op/TrackOperation.java b/org.spearce.egit.core/src/org/spearce/egit/core/op/TrackOperation.java
> index 6521f1c..af16cdb 100644
> --- a/org.spearce.egit.core/src/org/spearce/egit/core/op/TrackOperation.java
> +++ b/org.spearce.egit.core/src/org/spearce/egit/core/op/TrackOperation.java
> @@ -90,13 +90,20 @@ public class TrackOperation implements IWorkspaceRunnable {
>  							public boolean visit(IResource resource) throws CoreException {
>  								try {
>  									String repoPath = rm.getRepoRelativePath(resource);
> +									// We use add to reset the assume valid bit, so we check the bit
> +									// first. If a resource within a ignored folder is marked
> +									// we ignore it here, i.e. there is no way to unmark it expect
> +									// by explicitly selecting and invoking track on it.
>  									if (resource.getType() == IResource.FILE) {
>  										Entry entry = index.getEntry(repoPath);
> -										if (!Team.isIgnored((IFile)resource) || entry != null && entry.isAssumedValid()) {
> +										if (!Team.isIgnoredHint(resource) || entry != null && entry.isAssumedValid()) {
>  											entry = index.add(rm.getWorkDir(), new File(rm.getWorkDir(), repoPath));
>  											entry.setAssumeValid(false);
>  										}
>  									}
> +									if (Team.isIgnoredHint(resource))
> +										return false;
> +
>  								} catch (IOException e) {
>  									e.printStackTrace();
>  									throw Activator.error(CoreText.AddOperation_failed, e);

-- 
Galder Zamarreño
Sr. Software Maintenance Engineer
JBoss, a division of Red Hat

Re: [egit-jgit] excluded patterns are decorated as being untracked

[Robin Rosenberg]

onsdagen den 18 juni 2008 17.40.56 skrev Galder Zamarreno:
> Hmmmm, is marking a resource as "derived" recursive? i.e. if I mark 
> "output" folder as derived, would anything under it be considered 
> derived? It'd be a pain to go a mark as derived each and every class.

It seems it is not recursive and worse, not saved with the project.

> I suppose you still need the patch to use Team.isIgnoredHint to get 
> derived resources to be ignored, correct?

You'll need another patch for the decorator to pick up derived resources
properly. Probably not the final solution to decorations. For one thing the
decorators are horribly inefficient.

> Hmmmm, so the proper way is either marking resources as derived and use 
> your patch or implementing .git/info/exclude, correct?

I'd say implementing .git/info/exlude is the proper solution. The others are
mere workarounds.

-- robin

>From 7acc0e63886ed8eda6b38a5edbfe9a6aa4d509dc Mon Sep 17 00:00:00 2001
From: Robin Rosenberg <robin.rosenberg@dewire.com>
Date: Wed, 18 Jun 2008 23:50:42 +0200
Subject: [PATCH] Decorate derived resources as ignored.

This is done by using the appropriate API.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../internal/decorators/GitResourceDecorator.java  |    3 +--
 1 files changed, 1 insertions(+), 2 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 4b6394c..0308f6a 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
@@ -273,8 +273,7 @@ public class GitResourceDecorator extends LabelProvider implements
 							}
 
 						} else {
-							if (rsrc.getType() == IResource.FILE
-									&& Team.isIgnored((IFile) rsrc)) {
+							if (Team.isIgnoredHint(rsrc)) {
 								decoration.addSuffix("(ignored)");
 							} else {
 								decoration.addPrefix(">");
-- 
1.5.5.1.178.g1f811