Re: [EGIT PATCH] Add support for writing/appending .gitignore file

[Robin Rosenberg <robin.rosenberg.lists@dewire.com>]

First, Ferry Huberts is also working on a solution for ignore See 
http://thread.gmane.org/gmane.comp.version-control.git/114825 though you 
focus on different aspects.

söndag 19 april 2009 15:09:32 skrev Alex Blewitt <alex.blewitt@gmail.com>:
> This is in addition to the other patches mailed earlier and attached  
> with issue 64

This patch is whitespace damaged.  Pasting into gmail won't work. Gmail
has authenticated SMTP on port 25 and 465 (SSL) so git-send-email should work that way.

> +	private static final String GITIGNORE_ENCODING = "UTF-8";

For the time being we use Constants.CHARACTER_ENCODING (which
is UTF-8). There are some problems with that too. We read an
interpret (by guessing) (using RawParseUtils.decode) and write UTF-8 (always).
This is one of git's weak spots; it doesn't define encoding of stuff. For
JGit we do (it's perfectly valid since it's not defined). Our internal
encoding is UTF-8, with fallbacks for accepting other encodings if
it doesn't look like UTF-8. See RawParseUtils.decode() for details.

You may also want to look at msysgit's list of encoding related bug reports.

> +	private static final String GITIGNORE = ".gitignore";
> +
>   	@SuppressWarnings("restriction")
>   	@Override
>   	public void run(IAction action) {
> -
> +		NullProgressMonitor m = new NullProgressMonitor();
I guess this method should execute fairly fast, but in general we should run
with a real progress monitor. See an action, like Track (maybe we should
rename to TrackAction...).

	getTargetPart().getSite().getWorkbenchWindow().run

>   		IResource[] resources = getSelectedResources();
> -		for (IResource resource : resources) {
> -			// NB This does the same thing in DecoratableResourceAdapter, but  
> neither currently consult .gitignore
> -			if (!Team.isIgnoredHint(resource))
> -			{
> -				// TODO Actually add to .gitignore here

I think this series should be one patch only.

> +		try {
> +			for (IResource resource : resources) {
> +				// TODO This is pretty inefficient; multiple ignores in the same  
> directory cause multiple writes.
> +				// NB This does the same thing in DecoratableResourceAdapter, but  
> neither currently consult .gitignore
> +				if (!Team.isIgnoredHint(resource)) {
> +					IContainer container = resource.getParent();
> +					IFile gitignore = container.getFile(new Path(GITIGNORE));
> +					String entry = "/" + resource.getName() + "\n"; //$NON-NLS-1$  // 
> $NON-NLS-2$
> +					// TODO What is the character set and new-line convention?
> +					if(gitignore.exists()) {
> +						// This is ugly. CVS uses an internal representation of  
> the .gitignore to re-write/overwrite each time.
> +						ByteArrayOutputStream out = new ByteArrayOutputStream(2048);
> +						out.write(entry.getBytes(GITIGNORE_ENCODING)); // TODO Default  
> encoding?
> +						gitignore.appendContents(new  
> ByteArrayInputStream(out.toByteArray()),true,true,m);
> +					} else {
> +						ByteArrayInputStream bais = new  
> ByteArrayInputStream( entry.getBytes(GITIGNORE_ENCODING) ); //$NON- 
> NLS-1$
> +						gitignore.create( bais,true,m);					
> +					}
The character encoding issues, I think, should be interpreted such that we rewrite the whole
file in the same encoding, should it actually change.

> +				}
>   			}
> +		} catch (UnsupportedEncodingException e) {
> +			// TODO Auto-generated catch block
> +			e.printStackTrace();
> +		} catch (CoreException e) {
> +			// TODO Auto-generated catch block
> +			e.printStackTrace();
> +		} catch (IOException e) {
> +			// TODO Auto-generated catch block
> +			e.printStackTrace();

Some actual error logging would be nice. Activator.logError for just logging and MessageDialog.openError
for posting an error message to the user.

>   		}
>   		return;
>   	}

-- robin