Re: [EGIT PATCH] Add support for writing/appending .gitignore file
- From
Robin Rosenberg <robin.rosenberg.lists@dewire.com>
- Date
- Apr 19, 2009, 21:50 UTC
- Message-ID
- <200904192350.56348.robin.rosenberg.lists@dewire.com>
- In-Reply-To
- <AFFAB806-28F7-4D48-B603-B7B96052B0F3@gmail.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.
Show 7 quoted lines
> + 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
Show 7 quoted lines
> 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 hereI think this series should be one patch only.
Show 26 quoted lines
> + 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.
Show 11 quoted lines
> + }
> }
> + } 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