Bug 94987

Comment 1 Mike Wilson 2005-05-12 12:28:57 EDT

Note that this is on the high priority polish items list for R3.1.

Comment 2 Dani Megert 2005-05-17 13:30:32 EDT

New API is needed for this in Platform Text, see attached patch.
Erich, can you please review.

Comment 3 Dani Megert 2005-05-17 13:31:32 EDT

Created attachment 21277 [details]
API additions to org.eclipse.text

Comment 4 David Williams 2005-05-17 14:04:50 EDT

I'm not writing this as an objection, but "food for thought" ... is the proposed
"ILineDelimiterProvider" expected to be used/implemented elsewhere? I'm
wondering why its not IDocumentExtension5 instead? This would better signify
that this is a "normal" part of an IDocument. (If that is indeed the intent). We
have (always) something similar in IStructuredDocument, which is why I'm asking. 

Comment 5 Dani Megert 2005-05-17 17:02:15 EDT

Thanks David, your input is always welcome. I thought for quite a while about
this and the reason for the proposed API change is that a document/IDocument
really doesn't care about a default line delimiter because it can handle mixed
line delimiters. Therefore I did not want to mixin this new behavior with an
IDocumentExtension which would indicate that it belonged to the document. Does
this make sense?

Comment 6 David Williams 2005-05-17 20:50:55 EDT

Yes, makes sense, and I could live with that name. Thinking about it now,
realize we call ours "preferredLineDelimiter" (instead of "default"), as a
message to clients that while the IDocument itself doesn't care (and can handle
many types), but if the client cares, then they should use 'preferred' (e.g.
when saving, creating an empty document based off another exemplar document,
etc.). Not sure if this is exactly the case you are trying to cover, but I do
see "preferredLineDeliminter" as a correct 'Document' concept (even though it can 
handle other existing line deliminters in the content itself), it seems natural
to me to think of it as having a preferred one, when a client needs to pick one
to use with that document. I know its customary in some text/platform code to
use getLineDeliminter(1) to see what's "currently in use", but I'd advocate
those should typically be getPreferredLineDelimiter() [which might default to
current "what's on first line" if not otherwise specfified.]

Comment 7 Erich Gamma 2005-05-18 03:09:42 EDT

My only nit with the name is that we usually use *Provider for something that
you plug-in into another object (e.g. content provider, label provider). This
isn't the case here, in this case it is an interface addition. 

Given all the discussions on line delimiter I have a hard time to come to the
conclusion that line delimiter handling isn't a key responsibility of IDocument.
I'm therefore leaning toward an interface extension name.

Dani has the final decision and the addition of this interface is approved. 

Also I will be traveling and will not be able to further comment within the next
day.

Comment 8 Dani Megert 2005-05-18 04:17:41 EDT

>The proposed 'getPreferredLineDelimiter() is currently provided via 
>but I'd advocate those should typically be getPreferredLineDelimiter()

For this we already have TextUtilities.getDefaultLineDelimiter(IDocument) and it
is speced like you just proposed, therefore I would stick to
'get/setDefaultLineDelimiter. All current clients that want to prevent mixed
line delimiters in Document use (or should use)
TextUtilities.getDefaultLineDelimiter(IDocument) which in the future will return
the provided line delimiter for documents that have no line delimiter yet and
which implement ILineDelimiterProvider (or IDocumentExtension4, see below).

Erich's point about the name itself is a good one and after looking at IDocument
again, which already has getLegalLineDelimiters() and getLineDelimiter(int), I
decided to add the new methods to IDocumentExtension4 (i.e. get rid of
ILineDelimiterProvider). TextUtilities.getDefaultLineDelimiter(IDocument) will
call document.getDefaultLineDelimiter() for documents that implement
IDocumentExtension4.

There's another benefit for this solution: clients will see that there's such a
method while TextUtilities.getDefaultLineDelimiter(IDocument) might not be that
visible.

Comment 9 Dani Megert 2005-05-19 12:45:40 EDT

Fixed in HEAD.

In order to be more expressive I called the setter setInitialLineDelimiter()
instead of setDefaultLineDelimiert().

Updated AbstractDocument, TextUtilities, File Buffers plug-in and
FileDocumentProvider.

For a verification I've tested the Text and Java editors and also the example
Template editor which uses a FileDocumentProvider instead of file buffers. I
also ran all our tests with a wrapper that checks each resource (and its
children) being deleted for mixed line delimiters ==> no failures (though  those
would probably have existed before my changes ;-).

Comment 10 Dani Megert 2005-05-19 12:45:50 EDT

.

Comment 11 Mike Wilson 2005-05-19 13:59:20 EDT

Can you attach the final version of the API to this bug?

Comment 12 Dani Megert 2005-05-19 15:34:23 EDT

IDocumentExtension4

	/**
	 * Returns this document's default line delimiter.
	 * <p>
	 * This default line delimiter should be used by clients who
	 * want unique delimiters (e.g. 'CR's) in the document.</p> 
	 *
	 * @return the default line delimiter or <code>null</code> if none
	 */
	String getDefaultLineDelimiter();
	
	/**
	 * Sets this document's initial line delimiter i.e. the one
	 * which is returned by <code>getDefaultLineDelimiter</code>
	 * if the document does not yet contain any line delimiter.
	 *
	 * @param lineDelimiter the default line delimiter
	 */
	void setInitialLineDelimiter(String lineDelimiter);

Comment 13 Tom Hofmann 2005-05-30 12:38:43 EDT

verified line-delimiter awareness in the RC1 test pass.

checked that the correct delimiter is picked up both for existing and new/empty
files:
- by refactorings (move member type)
  - see bug 97198 for a missing case
- when editing
- when pasting
- template expansion
- new type wizards