Home » Archived » IMP » Proposed change to IParseController API
| Proposed change to IParseController API [message #656452] |
Fri, 25 February 2011 17:55  |
 Robert M. Fuhrer
Messages: 294
Registered: July 2009 |
Senior Member |
|
|
Hi out there,
I'd like to make the following mostly-backward-compatible enhancement to the IParseController
interface. Specifically, I'd like to add the following two methods to the API:
Object parse(IDocument doc, IProgressMonitor monitor);
IDocument getDocument();
Note that I'm not proposing to remove the current method:
Object parse(String sourceText, IProgressMonitor monitor);
The motivations for this change:
- Coordination between open editors and the indexing mechanism (in org.eclipse.imp.pdb)
is easier to manage if an IDocument is available, in lieu of the raw source text.
- An IDocument provides a number of other services (like offset->line mappings) that
clients might find useful.
I've reviewed all callers of the current IParseController.parse(String, IProgressMonitor)
method, and nearly all of them have an IDocument, and so they look like this:
parseController.parse(document.get(), monitor);
Since the existing parse(String, IProgressMonitor) interface method will remain, the few
clients that don't have an IDocument handy will still work. Moreover, any clients of the
proposed getDocument() method will occur in contexts where an IDocument will have been
available when parse() is called, so getDocument() will return something useful.
Finally, in terms of IParseController implementation burden, it's trivial to add support
for this new API to the existing base class ParseControllerBase:
public class ParseControllerBase {
// ...
private IDocument fDocument;
public Object parse(IDocument doc, IProgressMonitor mon) {
fDocument = doc;
return parse(fDocument.get(), mon);
}
public IDocument getDocument() {
return fDocument;
}
}
so the vast majority of implementers, if not all of them, will have nothing to do.
Any objections?
--
Cheers,
-- Bob
--------------------------------
Robert M. Fuhrer
Research Staff Member
Programming Technologies Dept.
IBM T.J. Watson Research Center
IDE Meta-tooling Platform Project Lead (http://www.eclipse.org/imp)
X10: Productive High-Performance Parallel Programming (http://x10.sf.net)
|
|
|
| Re: Proposed change to IParseController API [message #658014 is a reply to message #656452] |
Sat, 05 March 2011 01:44  |
Brian Payton
Messages: 154
Registered: July 2009 |
Senior Member |
|
|
Hi Bob,
The Parse Controller's main job appears to be to associate a lexer and
parser, parse a source string, and return the generated AST. That is a
generally useful concept for parsers. I would like to have a Parse
Controller for our parsers, even though we're not (yet) using all the
other IMP infrastructure.
Would it be possible to refactor the IParseController interface so that
there is a base version that has very few dependencies (just basic Java
and Eclipse stuff), then a more elaborate IMP-aware version that builds
on that? That way, we could use the Parse Controller interface for our
"raw" parser needs now as well as allowing us to integrate with
IMP-based functionality in the future.
On 2/25/2011 9:55 AM, Robert M. Fuhrer wrote:
> Hi out there,
>
> I'd like to make the following mostly-backward-compatible enhancement to
> the IParseController
> interface. Specifically, I'd like to add the following two methods to
> the API:
>
> Object parse(IDocument doc, IProgressMonitor monitor);
> IDocument getDocument();
>
> Note that I'm not proposing to remove the current method:
>
> Object parse(String sourceText, IProgressMonitor monitor);
>
> The motivations for this change:
> - Coordination between open editors and the indexing mechanism (in
> org.eclipse.imp.pdb)
> is easier to manage if an IDocument is available, in lieu of the raw
> source text.
> - An IDocument provides a number of other services (like offset->line
> mappings) that
> clients might find useful.
>
> I've reviewed all callers of the current IParseController.parse(String,
> IProgressMonitor)
> method, and nearly all of them have an IDocument, and so they look like
> this:
>
> parseController.parse(document.get(), monitor);
>
> Since the existing parse(String, IProgressMonitor) interface method will
> remain, the few
> clients that don't have an IDocument handy will still work. Moreover,
> any clients of the
> proposed getDocument() method will occur in contexts where an IDocument
> will have been
> available when parse() is called, so getDocument() will return something
> useful.
>
> Finally, in terms of IParseController implementation burden, it's
> trivial to add support
> for this new API to the existing base class ParseControllerBase:
>
> public class ParseControllerBase {
> // ...
> private IDocument fDocument;
>
> public Object parse(IDocument doc, IProgressMonitor mon) {
> fDocument = doc;
> return parse(fDocument.get(), mon);
> }
>
> public IDocument getDocument() {
> return fDocument;
> }
> }
>
> so the vast majority of implementers, if not all of them, will have
> nothing to do.
>
> Any objections?
>
|
|
|
| Re: Proposed change to IParseController API [message #659234 is a reply to message #656452] |
Fri, 11 March 2011 15:45 |
Robert M. Fuhrer
Messages: 294
Registered: July 2009 |
Senior Member |
|
|
I'd like to amend the proposed API change, based on some feedback from Jurgen Vinju
(thanks!).
Specifically, he pointed out that there's no need to pass the IDocument to each and
every call to parse(...). There would be no real use for that anyway - the framework
would in fact pass the exact same IDocument instance every time.
So, instead, I propose that we modify IParseController.initialize() so that it takes
an IDocument, like so:
void initialize(IDocument doc, IPath filePath, ISourceProject project,
IMessageHandler handler);
This makes it clear that a given IParseController instance would be associated with
a specific IDocument throughout its life.
The originally proposed new flavor of parse(...) that takes an IDocument would be
replaced by one that takes just an IProgressMonitor:
Object parse(IProgressMonitor monitor);
The original parse(String, IProgressMonitor) would continue to exist, for clients
and contexts where an IDocument isn't available.
The base class implementation (ParseControllerBase) of parse(IProgressMonitor) would
simply call parse(doc.get(), monitor). Any implementation extending that class would
most likely require no changes.
In short, the interface would look like this (omitting unrelated methods):
public interface IParseController extends ILanguageService {
void initialize(IDocument doc, IPath filePath, ISourceProject project,
IMessageHandler handler);
IDocument getDocument();
Object parse(String source, IProgressMonitor mon);
/**
* Parses the document's current contents. This IParseController must have
* been initialized with a non-null IDocument. If not, it will throw an
* IllegalStateException.
*/
Object parse(IProgressMonitor mon);
}
On 2/25/11 12:55 PM, Robert M. Fuhrer wrote:
> Hi out there,
>
> I'd like to make the following mostly-backward-compatible enhancement to the IParseController
> interface. Specifically, I'd like to add the following two methods to the API:
>
> Object parse(IDocument doc, IProgressMonitor monitor);
> IDocument getDocument();
>
> Note that I'm not proposing to remove the current method:
>
> Object parse(String sourceText, IProgressMonitor monitor);
>
> The motivations for this change:
> - Coordination between open editors and the indexing mechanism (in org.eclipse.imp.pdb)
> is easier to manage if an IDocument is available, in lieu of the raw source text.
> - An IDocument provides a number of other services (like offset->line mappings) that
> clients might find useful.
>
> I've reviewed all callers of the current IParseController.parse(String, IProgressMonitor)
> method, and nearly all of them have an IDocument, and so they look like this:
>
> parseController.parse(document.get(), monitor);
>
> Since the existing parse(String, IProgressMonitor) interface method will remain, the few
> clients that don't have an IDocument handy will still work. Moreover, any clients of the
> proposed getDocument() method will occur in contexts where an IDocument will have been
> available when parse() is called, so getDocument() will return something useful.
>
> Finally, in terms of IParseController implementation burden, it's trivial to add support
> for this new API to the existing base class ParseControllerBase:
>
> public class ParseControllerBase {
> // ...
> private IDocument fDocument;
>
> public Object parse(IDocument doc, IProgressMonitor mon) {
> fDocument = doc;
> return parse(fDocument.get(), mon);
> }
>
> public IDocument getDocument() {
> return fDocument;
> }
> }
>
> so the vast majority of implementers, if not all of them, will have nothing to do.
>
> Any objections?
>
--
Cheers,
-- Bob
--------------------------------
Robert M. Fuhrer
Research Staff Member
Programming Technologies Dept.
IBM T.J. Watson Research Center
IDE Meta-tooling Platform Project Lead (http://www.eclipse.org/imp)
X10: Productive High-Performance Parallel Programming (http://x10.sf.net)
|
|
|
| Re: Proposed change to IParseController API [message #659243 is a reply to message #658014] |
Fri, 11 March 2011 15:47 |
Robert M. Fuhrer
Messages: 294
Registered: July 2009 |
Senior Member |
|
|
On 3/4/11 8:44 PM, Brian Payton wrote:
> Hi Bob,
>
> The Parse Controller's main job appears to be to associate a lexer and parser, parse a source string, and return the
> generated AST. That is a generally useful concept for parsers. I would like to have a Parse Controller for our parsers,
> even though we're not (yet) using all the other IMP infrastructure.
To be honest, I'd rather not have other clients (including the IMP runtime) have to
do instanceof tests to determine whether the IDocument support is available. I'm not
really a big fan of extension interfaces, though I do see their use in preserving
backward compatibility.
Are you concerned about additional dependencies? IDocument is a pretty basic Eclipse
interface, and it resides in a plugin (org.eclipse.jface.text) that the IMP runtime
already depended on.
Your implementation can safely ignore the additional API method, getDocument(). Any
clients you have can simply not call the variant of parse(...) that requires an
IDocument.
Also, when you start using the rest of the IMP framework, the framework *will* do the
right thing if your IParseController implementation doesn't store the IDocument passed
in. The only thing that *won't* happen is that the indexer mechanism won't be able to
track changes in open editor buffers. But it will handle that case gracefully (indexing
based on resource changes will happen as usual).
Oh, and see my other post today on the subject.
> Would it be possible to refactor the IParseController interface so that there is a base version that has very few
> dependencies (just basic Java and Eclipse stuff), then a more elaborate IMP-aware version that builds on that? That way,
> we could use the Parse Controller interface for our "raw" parser needs now as well as allowing us to integrate with
> IMP-based functionality in the future.
>
> On 2/25/2011 9:55 AM, Robert M. Fuhrer wrote:
>> Hi out there,
>>
>> I'd like to make the following mostly-backward-compatible enhancement to
>> the IParseController
>> interface. Specifically, I'd like to add the following two methods to
>> the API:
>>
>> Object parse(IDocument doc, IProgressMonitor monitor);
>> IDocument getDocument();
>>
>> Note that I'm not proposing to remove the current method:
>>
>> Object parse(String sourceText, IProgressMonitor monitor);
>>
>> The motivations for this change:
>> - Coordination between open editors and the indexing mechanism (in
>> org.eclipse.imp.pdb)
>> is easier to manage if an IDocument is available, in lieu of the raw
>> source text.
>> - An IDocument provides a number of other services (like offset->line
>> mappings) that
>> clients might find useful.
>>
>> I've reviewed all callers of the current IParseController.parse(String,
>> IProgressMonitor)
>> method, and nearly all of them have an IDocument, and so they look like
>> this:
>>
>> parseController.parse(document.get(), monitor);
>>
>> Since the existing parse(String, IProgressMonitor) interface method will
>> remain, the few
>> clients that don't have an IDocument handy will still work. Moreover,
>> any clients of the
>> proposed getDocument() method will occur in contexts where an IDocument
>> will have been
>> available when parse() is called, so getDocument() will return something
>> useful.
>>
>> Finally, in terms of IParseController implementation burden, it's
>> trivial to add support
>> for this new API to the existing base class ParseControllerBase:
>>
>> public class ParseControllerBase {
>> // ...
>> private IDocument fDocument;
>>
>> public Object parse(IDocument doc, IProgressMonitor mon) {
>> fDocument = doc;
>> return parse(fDocument.get(), mon);
>> }
>>
>> public IDocument getDocument() {
>> return fDocument;
>> }
>> }
>>
>> so the vast majority of implementers, if not all of them, will have
>> nothing to do.
>>
>> Any objections?
>>
--
Cheers,
-- Bob
--------------------------------
Robert M. Fuhrer
Research Staff Member
Programming Technologies Dept.
IBM T.J. Watson Research Center
IDE Meta-tooling Platform Project Lead (http://www.eclipse.org/imp)
X10: Productive High-Performance Parallel Programming (http://x10.sf.net)
|
|
|
| Re: Proposed change to IParseController API [message #659305 is a reply to message #659243] |
Sat, 12 March 2011 00:49 |
Brian Payton
Messages: 154
Registered: July 2009 |
Senior Member |
|
|
Yes, I was concerned about the added dependency. We have a client who
wants to use our parsers while dragging as little of the rest of Eclipse
along as possible.
That said, I can certainly see that value of being able to use an
IDocument in the parser interface.
On 3/11/2011 7:47 AM, Robert M. Fuhrer wrote:
> On 3/4/11 8:44 PM, Brian Payton wrote:
>> Hi Bob,
>>
>> The Parse Controller's main job appears to be to associate a lexer and
>> parser, parse a source string, and return the
>> generated AST. That is a generally useful concept for parsers. I would
>> like to have a Parse Controller for our parsers,
>> even though we're not (yet) using all the other IMP infrastructure.
>
> To be honest, I'd rather not have other clients (including the IMP
> runtime) have to
> do instanceof tests to determine whether the IDocument support is
> available. I'm not
> really a big fan of extension interfaces, though I do see their use in
> preserving
> backward compatibility.
>
> Are you concerned about additional dependencies? IDocument is a pretty
> basic Eclipse
> interface, and it resides in a plugin (org.eclipse.jface.text) that the
> IMP runtime
> already depended on.
>
> Your implementation can safely ignore the additional API method,
> getDocument(). Any
> clients you have can simply not call the variant of parse(...) that
> requires an
> IDocument.
>
> Also, when you start using the rest of the IMP framework, the framework
> *will* do the
> right thing if your IParseController implementation doesn't store the
> IDocument passed
> in. The only thing that *won't* happen is that the indexer mechanism
> won't be able to
> track changes in open editor buffers. But it will handle that case
> gracefully (indexing
> based on resource changes will happen as usual).
>
> Oh, and see my other post today on the subject.
>
>> Would it be possible to refactor the IParseController interface so
>> that there is a base version that has very few
>> dependencies (just basic Java and Eclipse stuff), then a more
>> elaborate IMP-aware version that builds on that? That way,
>> we could use the Parse Controller interface for our "raw" parser needs
>> now as well as allowing us to integrate with
>> IMP-based functionality in the future.
>>
>> On 2/25/2011 9:55 AM, Robert M. Fuhrer wrote:
>>> Hi out there,
>>>
>>> I'd like to make the following mostly-backward-compatible enhancement to
>>> the IParseController
>>> interface. Specifically, I'd like to add the following two methods to
>>> the API:
>>>
>>> Object parse(IDocument doc, IProgressMonitor monitor);
>>> IDocument getDocument();
>>>
>>> Note that I'm not proposing to remove the current method:
>>>
>>> Object parse(String sourceText, IProgressMonitor monitor);
>>>
>>> The motivations for this change:
>>> - Coordination between open editors and the indexing mechanism (in
>>> org.eclipse.imp.pdb)
>>> is easier to manage if an IDocument is available, in lieu of the raw
>>> source text.
>>> - An IDocument provides a number of other services (like offset->line
>>> mappings) that
>>> clients might find useful.
>>>
>>> I've reviewed all callers of the current IParseController.parse(String,
>>> IProgressMonitor)
>>> method, and nearly all of them have an IDocument, and so they look like
>>> this:
>>>
>>> parseController.parse(document.get(), monitor);
>>>
>>> Since the existing parse(String, IProgressMonitor) interface method will
>>> remain, the few
>>> clients that don't have an IDocument handy will still work. Moreover,
>>> any clients of the
>>> proposed getDocument() method will occur in contexts where an IDocument
>>> will have been
>>> available when parse() is called, so getDocument() will return something
>>> useful.
>>>
>>> Finally, in terms of IParseController implementation burden, it's
>>> trivial to add support
>>> for this new API to the existing base class ParseControllerBase:
>>>
>>> public class ParseControllerBase {
>>> // ...
>>> private IDocument fDocument;
>>>
>>> public Object parse(IDocument doc, IProgressMonitor mon) {
>>> fDocument = doc;
>>> return parse(fDocument.get(), mon);
>>> }
>>>
>>> public IDocument getDocument() {
>>> return fDocument;
>>> }
>>> }
>>>
>>> so the vast majority of implementers, if not all of them, will have
>>> nothing to do.
>>>
>>> Any objections?
>>>
>
>
|
|
|
| Re: Proposed change to IParseController API [message #660470 is a reply to message #659305] |
Fri, 18 March 2011 14:43 |
Robert M. Fuhrer
Messages: 294
Registered: July 2009 |
Senior Member |
|
|
On 3/11/11 7:49 PM, Brian Payton wrote:
> Yes, I was concerned about the added dependency. We have a client who wants to use our parsers while dragging as little
> of the rest of Eclipse along as possible.
Well, there are actually no additional dependencies, neither at the plugin
level, nor at the interface level. (IRegion, which is used in the signature
of the getTokenIterator() method, is in the same plugin/package as IDocument.)
So hopefully this shouldn't cause you or your clients any problems. But please
let me know if somehow this analysis isn't quite right.
> That said, I can certainly see that value of being able to use an IDocument in the parser interface.
>
> On 3/11/2011 7:47 AM, Robert M. Fuhrer wrote:
>> On 3/4/11 8:44 PM, Brian Payton wrote:
>>> Hi Bob,
>>>
>>> The Parse Controller's main job appears to be to associate a lexer and
>>> parser, parse a source string, and return the
>>> generated AST. That is a generally useful concept for parsers. I would
>>> like to have a Parse Controller for our parsers,
>>> even though we're not (yet) using all the other IMP infrastructure.
>>
>> To be honest, I'd rather not have other clients (including the IMP
>> runtime) have to
>> do instanceof tests to determine whether the IDocument support is
>> available. I'm not
>> really a big fan of extension interfaces, though I do see their use in
>> preserving
>> backward compatibility.
>>
>> Are you concerned about additional dependencies? IDocument is a pretty
>> basic Eclipse
>> interface, and it resides in a plugin (org.eclipse.jface.text) that the
>> IMP runtime
>> already depended on.
>>
>> Your implementation can safely ignore the additional API method,
>> getDocument(). Any
>> clients you have can simply not call the variant of parse(...) that
>> requires an
>> IDocument.
>>
>> Also, when you start using the rest of the IMP framework, the framework
>> *will* do the
>> right thing if your IParseController implementation doesn't store the
>> IDocument passed
>> in. The only thing that *won't* happen is that the indexer mechanism
>> won't be able to
>> track changes in open editor buffers. But it will handle that case
>> gracefully (indexing
>> based on resource changes will happen as usual).
>>
>> Oh, and see my other post today on the subject.
>>
>>> Would it be possible to refactor the IParseController interface so
>>> that there is a base version that has very few
>>> dependencies (just basic Java and Eclipse stuff), then a more
>>> elaborate IMP-aware version that builds on that? That way,
>>> we could use the Parse Controller interface for our "raw" parser needs
>>> now as well as allowing us to integrate with
>>> IMP-based functionality in the future.
>>>
>>> On 2/25/2011 9:55 AM, Robert M. Fuhrer wrote:
>>>> Hi out there,
>>>>
>>>> I'd like to make the following mostly-backward-compatible enhancement to
>>>> the IParseController
>>>> interface. Specifically, I'd like to add the following two methods to
>>>> the API:
>>>>
>>>> Object parse(IDocument doc, IProgressMonitor monitor);
>>>> IDocument getDocument();
>>>>
>>>> Note that I'm not proposing to remove the current method:
>>>>
>>>> Object parse(String sourceText, IProgressMonitor monitor);
>>>>
>>>> The motivations for this change:
>>>> - Coordination between open editors and the indexing mechanism (in
>>>> org.eclipse.imp.pdb)
>>>> is easier to manage if an IDocument is available, in lieu of the raw
>>>> source text.
>>>> - An IDocument provides a number of other services (like offset->line
>>>> mappings) that
>>>> clients might find useful.
>>>>
>>>> I've reviewed all callers of the current IParseController.parse(String,
>>>> IProgressMonitor)
>>>> method, and nearly all of them have an IDocument, and so they look like
>>>> this:
>>>>
>>>> parseController.parse(document.get(), monitor);
>>>>
>>>> Since the existing parse(String, IProgressMonitor) interface method will
>>>> remain, the few
>>>> clients that don't have an IDocument handy will still work. Moreover,
>>>> any clients of the
>>>> proposed getDocument() method will occur in contexts where an IDocument
>>>> will have been
>>>> available when parse() is called, so getDocument() will return something
>>>> useful.
>>>>
>>>> Finally, in terms of IParseController implementation burden, it's
>>>> trivial to add support
>>>> for this new API to the existing base class ParseControllerBase:
>>>>
>>>> public class ParseControllerBase {
>>>> // ...
>>>> private IDocument fDocument;
>>>>
>>>> public Object parse(IDocument doc, IProgressMonitor mon) {
>>>> fDocument = doc;
>>>> return parse(fDocument.get(), mon);
>>>> }
>>>>
>>>> public IDocument getDocument() {
>>>> return fDocument;
>>>> }
>>>> }
>>>>
>>>> so the vast majority of implementers, if not all of them, will have
>>>> nothing to do.
>>>>
>>>> Any objections?
>>>>
>>
>>
--
Cheers,
-- Bob
--------------------------------
Robert M. Fuhrer
Research Staff Member
Programming Technologies Dept.
IBM T.J. Watson Research Center
IDE Meta-tooling Platform Project Lead (http://www.eclipse.org/imp)
X10: Productive High-Performance Parallel Programming (http://x10.sf.net)
|
|
|
| Re: Proposed change to IParseController API [message #661036 is a reply to message #660470] |
Tue, 22 March 2011 19:38 |
Brian Payton
Messages: 154
Registered: July 2009 |
Senior Member |
|
|
Looking at it again, the problem is not the IParseController interface
itself. My concern is with the plugin that it's packaged in,
org.eclipse.imp.runtime. Here's the plugins that are directly or
indirectly required by org.eclipse.imp.runtime:
com.ibm.icu
javax.servlet
org.eclipse.osgi
lpg.runtime.java
org.eclipse.ant.core
org.eclipse.compare
org.eclipse.compare.core
org.eclipse.core.commands
org.eclipse.core.contenttype
org.eclipse.core.databinding
org.eclipse.core.databinding.observable
org.eclipse.core.databinding.property
org.eclipse.core.expressions
org.eclipse.core.filebuffers
org.eclipse.core.filesystem
org.eclipse.core.filesystem.win32.x86
org.eclipse.core.jobs
org.eclipse.core.net
org.eclipse.core.net.win32.x86
org.eclipse.core.resources
org.eclipse.core.resources.win32.x86
org.eclipse.core.runtime
org.eclipse.core.runtime.compatibility.auth
org.eclipse.core.runtime.compatibility.registry
org.eclipse.core.variables
org.eclipse.debug.core
org.eclipse.debug.ui
org.eclipse.ecf
org.eclipse.ecf.identity
org.eclipse.ecf.filetransfer
org.eclipse.ecf.provider.filetransfer
org.eclipse.ecf.provider.filetransfer.ssl
org.eclipse.ecf.ssl
org.eclipse.equinox.app
org.eclipse.equinox.common
org.eclipse.equinox.concurrent
org.eclipse.equinox.p2.core
org.eclipse.equinox.p2.engine
org.eclipse.equinox.p2.metadata
org.eclipse.equinox.p2.metadata.repository
org.eclipse.equinox.p2.repository
org.eclipse.equinox.preferences
org.eclipse.equinox.registry
org.eclipse.equinox.security
org.eclipse.equinox.security.win32.x86
org.eclipse.equinox.weaving.hook
org.eclipse.help
org.eclipse.jface
org.eclipse.jface.databinding
org.eclipse.jface.text
org.eclipse.ltk.core.refactoring
org.eclipse.ltk.ui.refactoring
org.eclipse.swt
org.eclipse.swt.win32.win32.x86
org.eclipse.team.core
org.eclipse.team.ui
org.eclipse.text
org.eclipse.ui
org.eclipse.ui.console
org.eclipse.ui.editors
org.eclipse.ui.forms
org.eclipse.ui.ide
org.eclipse.ui.navigator
org.eclipse.ui.views
org.eclipse.ui.win32
org.eclipse.ui.workbench
org.eclipse.ui.workbench.compatibility
org.eclipse.ui.workbench.texteditor
org.eclipse.osgi.services
So making use of any interface or class in org.eclipse.imp.runtime is
going to drag a significant chunk of Eclipse.
Any suggestions?
On 3/18/2011 7:43 AM, Robert M. Fuhrer wrote:
> On 3/11/11 7:49 PM, Brian Payton wrote:
>> Yes, I was concerned about the added dependency. We have a client who
>> wants to use our parsers while dragging as little
>> of the rest of Eclipse along as possible.
>
> Well, there are actually no additional dependencies, neither at the plugin
> level, nor at the interface level. (IRegion, which is used in the signature
> of the getTokenIterator() method, is in the same plugin/package as
> IDocument.)
>
> So hopefully this shouldn't cause you or your clients any problems. But
> please
> let me know if somehow this analysis isn't quite right.
>
>> That said, I can certainly see that value of being able to use an
>> IDocument in the parser interface.
>>
>> On 3/11/2011 7:47 AM, Robert M. Fuhrer wrote:
>>> On 3/4/11 8:44 PM, Brian Payton wrote:
>>>> Hi Bob,
>>>>
>>>> The Parse Controller's main job appears to be to associate a lexer and
>>>> parser, parse a source string, and return the
>>>> generated AST. That is a generally useful concept for parsers. I would
>>>> like to have a Parse Controller for our parsers,
>>>> even though we're not (yet) using all the other IMP infrastructure.
>>>
>>> To be honest, I'd rather not have other clients (including the IMP
>>> runtime) have to
>>> do instanceof tests to determine whether the IDocument support is
>>> available. I'm not
>>> really a big fan of extension interfaces, though I do see their use in
>>> preserving
>>> backward compatibility.
>>>
>>> Are you concerned about additional dependencies? IDocument is a pretty
>>> basic Eclipse
>>> interface, and it resides in a plugin (org.eclipse.jface.text) that the
>>> IMP runtime
>>> already depended on.
>>>
>>> Your implementation can safely ignore the additional API method,
>>> getDocument(). Any
>>> clients you have can simply not call the variant of parse(...) that
>>> requires an
>>> IDocument.
>>>
>>> Also, when you start using the rest of the IMP framework, the framework
>>> *will* do the
>>> right thing if your IParseController implementation doesn't store the
>>> IDocument passed
>>> in. The only thing that *won't* happen is that the indexer mechanism
>>> won't be able to
>>> track changes in open editor buffers. But it will handle that case
>>> gracefully (indexing
>>> based on resource changes will happen as usual).
>>>
>>> Oh, and see my other post today on the subject.
>>>
>>>> Would it be possible to refactor the IParseController interface so
>>>> that there is a base version that has very few
>>>> dependencies (just basic Java and Eclipse stuff), then a more
>>>> elaborate IMP-aware version that builds on that? That way,
>>>> we could use the Parse Controller interface for our "raw" parser needs
>>>> now as well as allowing us to integrate with
>>>> IMP-based functionality in the future.
>>>>
>>>> On 2/25/2011 9:55 AM, Robert M. Fuhrer wrote:
>>>>> Hi out there,
>>>>>
>>>>> I'd like to make the following mostly-backward-compatible
>>>>> enhancement to
>>>>> the IParseController
>>>>> interface. Specifically, I'd like to add the following two methods to
>>>>> the API:
>>>>>
>>>>> Object parse(IDocument doc, IProgressMonitor monitor);
>>>>> IDocument getDocument();
>>>>>
>>>>> Note that I'm not proposing to remove the current method:
>>>>>
>>>>> Object parse(String sourceText, IProgressMonitor monitor);
>>>>>
>>>>> The motivations for this change:
>>>>> - Coordination between open editors and the indexing mechanism (in
>>>>> org.eclipse.imp.pdb)
>>>>> is easier to manage if an IDocument is available, in lieu of the raw
>>>>> source text.
>>>>> - An IDocument provides a number of other services (like offset->line
>>>>> mappings) that
>>>>> clients might find useful.
>>>>>
>>>>> I've reviewed all callers of the current
>>>>> IParseController.parse(String,
>>>>> IProgressMonitor)
>>>>> method, and nearly all of them have an IDocument, and so they look
>>>>> like
>>>>> this:
>>>>>
>>>>> parseController.parse(document.get(), monitor);
>>>>>
>>>>> Since the existing parse(String, IProgressMonitor) interface method
>>>>> will
>>>>> remain, the few
>>>>> clients that don't have an IDocument handy will still work. Moreover,
>>>>> any clients of the
>>>>> proposed getDocument() method will occur in contexts where an
>>>>> IDocument
>>>>> will have been
>>>>> available when parse() is called, so getDocument() will return
>>>>> something
>>>>> useful.
>>>>>
>>>>> Finally, in terms of IParseController implementation burden, it's
>>>>> trivial to add support
>>>>> for this new API to the existing base class ParseControllerBase:
>>>>>
>>>>> public class ParseControllerBase {
>>>>> // ...
>>>>> private IDocument fDocument;
>>>>>
>>>>> public Object parse(IDocument doc, IProgressMonitor mon) {
>>>>> fDocument = doc;
>>>>> return parse(fDocument.get(), mon);
>>>>> }
>>>>>
>>>>> public IDocument getDocument() {
>>>>> return fDocument;
>>>>> }
>>>>> }
>>>>>
>>>>> so the vast majority of implementers, if not all of them, will have
>>>>> nothing to do.
>>>>>
>>>>> Any objections?
>>>>>
>>>
>>>
>
>
|
|
|
Goto Forum:
Current Time: Fri Apr 26 15:05:26 GMT 2024
Powered by FUDForum. Page generated in 0.06275 seconds |
] 
Search
Help
Register
Login
Home
