This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 91893 - [Embedding] Provide Brace Matching in embedded sections
Summary: [Embedding] Provide Brace Matching in embedded sections
Status: RESOLVED FIXED
Alias: None
Product: editor
Classification: Unclassified
Component: -- Other -- (show other bugs)
Version: 6.x
Hardware: PC Linux
: P2 blocker (vote)
Assignee: Vitezslav Stejskal
URL:
Keywords: API
Depends on:
Blocks: 95126 89283 103017 105123
  Show dependency tree
 
Reported: 2007-01-05 14:01 UTC by Miloslav Metelka
Modified: 2007-11-05 13:44 UTC (History)
1 user (show)

See Also:
Issue Type: TASK
Exception Reporter:


Attachments
jVi command mode use case example (1.15 KB, image/png)
2007-05-18 03:12 UTC, err
Details
Proposed alternate behavior for "Forward" allowDir (3.98 KB, patch)
2007-05-21 23:17 UTC, err
Details | Diff
The Braces Matching SPI documentation (99.32 KB, application/octet-stream)
2007-05-29 07:35 UTC, Vitezslav Stejskal
Details
Commit log from the CVS server (18.85 KB, text/plain)
2007-06-07 10:11 UTC, Vitezslav Stejskal
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Miloslav Metelka 2007-01-05 14:01:10 UTC
The current infrastructure is not prepared for doing embedded sections aware
brace matching (includes higlighting matching brace and jumping to it with
"Ctrl+[").
Ideally there should be a new SPI for this because the old one is
ExtSyntaxSupport.getMatchingBlock() which is not xml-layer registerable. The
ExtSyntaxSupport should be deprecated and removed in the future.
Comment 1 Vitezslav Stejskal 2007-05-02 12:53:13 UTC
The new SPI should support requirements voiced in issue #95126 and help to
resolve problems with long running searches such as issue #103017.
Comment 2 Vitezslav Stejskal 2007-05-10 07:13:07 UTC
I am working on the SPI in contrib/EditorBracesMatching. The module now contains
near-to-review SPI with documentation, usable implementation of the
infrastructure behind the SPI and the default matcher that provides simple
matching for {[( and their counterparts. At the moment there is no navigation
between matching braces and the infrastructure operates in the default mode -
search in both directions with a lookahead to the beginning, resp. end of a line.

Anybody interested in having a look and commenting just open the project in
Netbeans, clean & build, Generate javadoc and run in a trunk build with:

ant -Dtryme.args="-J-Dnbeditor-no-HighlightBraceLayer=true" tryme

The nbeditor-no-HighlightBraceLayer switch will turn off the old highlighting
layer in ExtCaret.
Comment 3 err 2007-05-11 01:47:34 UTC
ER1: BracesMatcherFactory.createMatcher take a MatcherContext, not just
a Document. Is there an implication that the returned BracesMatcher might be
different depending on the offset within the document? Is createMatcher cheap?

ER2: The old findMatchingBlock had a concept of a simple/fast match. A conscious
decision to drop it?

ER3: The javadoc for BracesMatcher interrupt example has
   ...void findMatches... and return;
should be ...int[]... and return null;

ER4: What's an example of findMatches returning more than one match?

ER5: OffTopic, but related. ExtCaret could be refactored so that the algorithm
it used for finding a match is separate from highlighting the match. Then the
get/setMatchBraceOffset methods could be dropped.
Comment 4 Vitezslav Stejskal 2007-05-11 04:28:01 UTC
ER1: Yes, the matcher can be different in diferent parts of the same document.
That's the complexity brought in with embedded languages. For example, in a
plain HTML document the matcher will match HTML tags, but if there is <script/>
section with javascript inside it, the matcher will have to match braces when
the cursor is in this section. So, in fact the infrastructure will use a
diferent matchers inside the <script/> section and outside of it.

Creation should be cheap, typically something like this:

public class YourMatcherImplFactory implements MatcherFactory {
    public BracesMatcher createMatcher(MatcherContext context) {
        return new YourMatcherImpl(context);
    }
}

There should be nothing done in YourMatcherImpl's constructor, all the work
should happen in findOrigin and findMatches. The context passed in will never
change so YourMatcherImpl can preserve an internal state between those two calls.

ER2: Semiconcious :-). The job for BracesMatcher's implementors is simple - do
your best as fast as you can. We always want precise results as quickly as
possible. The infrastructure uses a special thread for calling matchers and if
it takes too long to deliver results, the task will eventually be cancelled,
when the cursor moves again. Perhaps a user didn't need to see the results
anyway. These searches should generally be pretty quick - less then 1 second. If
it takes longer and user is still interested in the results, they will have to
wait a bit long. If it takes too long to be useable then there is probably
something broken in the matcher itself.

ER3: I'll fix that, thanks.

ER4: I don't have any, but supporting multiple matches comes for free, so why
not to have this possibility. Even though it's probably not going to be used. In
general it could be used for things like mark occurences in the new java editor,
but the java guys have done it their own way.

ER5: This is not described in the docs, but basically I'd like to support a
client property that would control the search direction. So, for example for jVi
you could do something like

jep.putClientProperty("nbeditor-bracesmatching-search-direction", "backward")

and the infrastructure would pick that up and use it for matching done in that
component. Then we can drop the get/setMatchBraceOffset, I think. Is that ok for
jVi?
Comment 5 err 2007-05-11 05:34:50 UTC
>> ER5: ExtCaret refactored finding a match is
>> separate from highlighting the match
> ER5: support a client property that would control the search direction
Today in ExtCaret there's something like
    protected void updateMatchBrace() {
      int[] matchBlk;
      ...do stuf to set matchBlk acc'd to algorithm...
      ...establish highlight marks...
    }
I'm thinking that if it looked like
    protected void updateMatchBrace() {
      int[] matches = null;
      int[] origin = findMatchOrigin(factory, doc, offset);
      if(origin) {
        matches = braceMatcher.findMatches(context);
        highlightMatch(origin, matches);
      }
    }
    protected int[] findMatchOrigin(factory, doc, offset) {
      ...do stuf to set matchBlk acc'd to algorithm...
    }
    protected void highlightMatch(int[] origin, int[] matches) {
      ...set up the highlight marks...
    }
The details aren't right, but the idea is that since jvi already extends
ExtCaret. With something like this jvi can override findMatchOrigin and
implement the find start of match algorithm anyway it wants to.
Comment 6 Vitezslav Stejskal 2007-05-11 10:28:47 UTC
ER5: Umm, I don't want to do this. In fact, I don't want the braces matching
support to be tied to any particular Caret implementation at all. What are the
usecases in jVi in regards to braces matching? I thought it's just controlling
the direction in which findOrigin searches? Or perhaps also avoiding proximity
search (ie. looking just right next to a caret)? Anything else?
Comment 7 err 2007-05-11 18:43:26 UTC
The thought is that with access to BracesMatcherFactory jvi wouldn't need to be
tied to any particular algorithm. In particular, I'm planning to provide an
option to select between vim compatible matching and "extended platform
matching" (like what you've demo'd now). Also, jvi could also implement its own
jump-to-matching-brace action rather than using the one from the editor kit.

Currently, jvi uses the MATCH_BRACE_BEFORE,MATCH_BRACE_AFTER for complete vim
input,command mode compatibility. But I will definitely add jvi options to
select different match algorithms. It sounds like the bases will be covered by
the builtin algorithms, especially if there's difficulty or reasons to not allow
the algorithm to be plugged in.
Comment 8 Vitezslav Stejskal 2007-05-14 04:25:14 UTC
Ok, I think I understand. This might be a bit too general, but hopefully useful
explanantion. Firstly, jVi is not a typical client of editor APIs, because it
does not want to just plugin its customized services, but also (sometimes)
redefine the way how the editor infrastructure works. Historically we wanted to
support this and made the APIs very open, you can see that pertty much
everything in o.n.editor and other editor packages is public, can be subclassed
and overwritten. What really happend though was that tha API became cluttered
and unmaintainable. Plus it turned out that typical modules are not interested
in rewriting the infrastructure, thay just want to plugin services like code
completion, syntax coloring, code folding, etc tailored for their specific file
type. That's why we are moving away from the old APIs and rebuilding them in
more structured/modular and more restrictive way. That said our distant goal is
to deprecate the whole editor/lib and forbid modules to use it.

Now, closer to braces matching, if anything you would like to have for jVi is
beneficial for Netbeans in general then I don't see any reason why we should not
extend the infrastructure to cover jVi usecases. If things were really jVi
specific we could always think of providing private/friend backdoor for jVi
modules to tweak the default funtionality. 

Could you please summarize what those additional usecases/search algorithms are?

The jump-to-matching-brace action should be easily replacable by using
Settings.Filter and CUSTOM_ACTION_LIST. The algorithm for calling BMFs was
factored out to MasterMatcher class and we could change this class so that some
other module could provide its implementation through the default lookup (not a
public API, just backdoor, implementation dependency would be needed).
Comment 9 err 2007-05-15 06:58:49 UTC
This entry has:
A. summary of jvi use match brace use cases
B. general info on jvi, a perspective

A. summary of jvi use match brace use cases
0- for goto-match command, vi-vim scans in the forward direction until it
   encounters a "brace" object, then jumps to its match. This implies that
   "if(a){www} el|se {xxx}" and goto-match ends up on the final '}'. And I
   believe that the highlight and goto should use the same algorithm. vim only
   highlights when caret at brace. (I prefer new NetBeans behavior)
1- continue to support the simple (){}[] case as currently does. (and to tell
   the truth, it wouldn't matter that much if this case had to be left out)
2- jvi "platform" mode that uses the platform for everything (and beeps
   if the cursor doesn't move on goto-match). This will be the default jvi
   behavior, assuming that the platform will scan, looking for a brace, in the
   goto-match action.

But its not quite that simple...
a- Switching between MATCH_BRACE_BEFORE/AFTER (via property or whatever) is
   needed for the cases when the caret straddles a couple of braces.  It also
   afects where the caret ends up on a goto-match action.
   given: <li>|<b>text</b></li>
   BEFORE: highlights <li></li>, goto </li>|, goto <li>|
   AFTER: highlights <b></b>, goto </b|>, goto <b|>
   This follows the convention that goto-match on a multichar brace positions
   at the end of the brace.
b- Also, in the case where both directions have a brace, jvi wants to go in
   the forward direction. If this behavior is not an available platform
   option, then jvi can get hold of a factory, do a forward search, and do the
   jump. Issue is that I like having highlight and goto-match use the same
   algorithm.

I guess from an integration point of view there are three options.
- Use the platform as is, with whatever options
- disable the platform matching, jvi implement match and highlight.
- plugin match algorithm
The first is the simplest for me. The second is an worst-case situation, is it
even possible? It seems the only issue is what to do when "xxxx }  |  { yyy".

> The jump-to-matching-brace action should be easily replacable...
If jvi is going to implement a jump-to-matching, there's no real need to
replace the action; jvi doesn't bind keys to particular actions. It finds
actions, from editor or filesystem, to get an implmentation.  However, if jvi
were to create an implementation, it would probably replace the action for
consistency; it does layer on top of the existing bindings and a user might
use that binding as well as the jvi command.

My thinking about jump-match, is that it doesn't makes sense to replace it if
the highlight algorithm can't be replaced. When you invoke go to match, it
should be the same algorithm as highlightMatch.

B. general info on jvi, a perspective
> ...are not interested in rewriting the infrastructure, thay just want to
> plugin services like code completion, syntax coloring, code folding,
> etc tailored for their specific file type.
Yep, the general usecase for jvi is very different, it doesn't want to
provide services, it wants to use whatever services are appropriate for the
file being edited; also it doesn't really match the old APIs that much
either, it basically depends on and uses swing directly for navigation and
text modification. But it does want to provide access to IDE/platform
features, and when they match vim behavior it tries to use the actions in a
way that is compatible with or at least matches the spirit of vim; for
example, jvi does a very good imitiation of the vim tagstack facility,
including displaying stack, using the kit.gotoDeclarationAction and the goto
class dialog which it picks up from the filesystem (trying first the 60, then
55, action).  It doesn't want to rewrite all the actions, it will just use
them where possible; for example JumpNextAction, matchBraceAction. And it
will provide additional capabilities through existing actions, for example the
bookmark and build actions.

I understand, and agree, that NB does not want to provide a platform where
you can implement any editor, I can't believe its a winning proposition; how
much use have the old APIs gotton outside of netbeans?  Why hasn't there been
a vi-vim editor before? Even with the APIs its just too much work to
impmlement an editor and tie it to an IDE.

The difficulties for jvi come when the existing actions aren't quite right
and their implementation can't be copied (use non-public APIs), for exmple
some of the NB55 java source navigation; or it would be a lot of work to
re-implement the actions. Some of the problem is that I don't know the
platform too well, so more features become possible with time. Also, jvi will
usually require porting to a new version since it gets very intimate with the
system. Poking into places it doesn't belong...

So, thanks for you patience, I hope this was useful in understanding jvi.
I don't think much special needs to be done re: brace match for jvi,
    ant -Dtryme.args="-J-Dnbeditor-no-HighlightBraceLayer=true" tryme
looks pretty good to me.
Sometimes I try to go to far..., perhaps like the old editor lib APIs.

Comment 10 err 2007-05-15 07:08:18 UTC
Note that for jvi, BEFORE/AFTER are not a direction as much as what character
to consider under the caret. So a couple more examples:
   given: <li>| <b>text</b></li>
   BEFORE: highlights <li></li>, goto </li>|, goto <li>|
   AFTER: highlights <b></b>, goto </b|>, goto <b|>
   Because if AFTER, no brace under caret, does forward search

   given: <li> |<b>text</b></li>
   BEFORE: no brace before, so do a forward search (same as AFTER)
   AFTER: highlights <b></b>, goto </b|>, goto <b|>
Comment 11 Vitezslav Stejskal 2007-05-17 07:28:24 UTC
Ok, thanks for the detailed answer. If I get it right jVi needs two modes that
you call BEFORE/AFTER, which basically work like that:

BEFORE: look right before the caret, if you find brace object match/show it, if
there is no brace object before the caret do the forward search

AFTER: do the forward search, which means match/show any braces object you may
find on a line from the caret to the end of the line

I think this can easily be mapped to what the bracesmatching module provides and
controlled by the nbeditor-bracesMatching-allowedSearchDirection property -
jVi's AFTER mode is the same as allowedSearchDirection == 'forward'. The BEFORE
mode is a bit trickier, but pretty much the reverse to what the module was doing
by default when the property was not set. I've formalized this behavior and
added a few more values for the allowedSearchDirection property. They are:

'backward', 'forward' - simple, searches in the given direction
'backard-preferred', 'forward-preferred' - basically searches in both direction,
but preferring one over the other if there are brace objects on both sides of
the caret
'both' - searches in both directions, but shows nothing if there are brace
objects on both sides

The allowedSearchDirection == 'forward-preferred' should be the one for jVi's
BEFORE mode. The detailed description can be found in javadoc, there is a
section with the usecases in the description of the o.n.spi.e.bracesmatching
package.

Also, I added a special action bound to ctrl-alt-shift-[ that shows a little
control panel allowing to change the value of the allowedSearchDirection
property for the text component, where the action was invoked from. It's good
for playing around with all the supported modes.

Speaking of the module's extensibility:

> - Use the platform as is, with whatever options
> - disable the platform matching, jvi implement match and highlight.
> - plugin match algorithm
> The first is the simplest for me. The second is an worst-case situation,
> is it even possible?

All three options are posible. For the second option (2) you would have to
filter out the highlighting layer supplied by the bracesmatching module and add
your own for jVi, you would also have to replace the jump-to-matching-brace
action. The third case would need some relatively simple changes in the
bracesmatching module, if it was ever needed.

I'd like to wrap up this discussion and ask for the API review. Before that I'd
like to ask you two questions.

1. Should you ever need to replace the whole module as in (2), do you think that
the information provided by BracesMatcher would be enough for your own algorithm
to work? In other words does the SPI require implementors to provide enough
information?

2. Do the new values available for the 'allowedSearchDirection' property cover
jVi usecases?

Thanks a lot.
Comment 12 err 2007-05-18 03:09:58 UTC
I think there's confusion with the meaning of AFTER/BEFORE. Considering that
the caret points between two characters, AFTER/BEFORE indicate which character
is important or "focused" for the purpose of brace highlighting and goto-match.

vi/vim are modal. In command mode the caret is a block, in input mode an
I-beam (vertical bar). The attached Block-AFTER.png shows command mode; this
is running on NB55 patched with the patch from issue 95126 (patch modified for
NB56). In the attached example, a matchBraceAction positions the caret such
that the block is on the matching brace, ')' in this example; note that this
means the ')' is AFTER the caret. So AFTER/BEFORE affect the caret positioning
in a matchBraceAction. In input mode, the character BEFORE the caret is
important; this is the same as netbeans.

So AFTER/BEFORE is orthogonal to the allowedSearchDirection property.

About the API/SPI.

> 1. [is] the ... BracesMatcher ... enough for your own algorithm
Yes.

> 2. values ... for the 'allowedSearchDirection' property cover jVi use cases?
allowedSearchDirection specifies the algorithm, it does cover the use cases.
However, jVi needs to specify the focusBias, either after/before.


API issues (probably just doc issues)

BracesMatcher.find{Origin,Matches} returns "null if the matcher can't detect
the origin" or "null if no matching areas can be found". But they both say
"BadLocationException - If the search fails." which sound like the same thing
as why null is returned, and so is confusing. (Well I wasn't really confused,
but ...).

In the example in BracesMatcher, syntactically needs "return null". But
semantically should it be "throw InterruptedException"? I guess it doesn't
really matter.
Comment 13 err 2007-05-18 03:12:17 UTC
Created attachment 42524 [details]
jVi command mode use case example
Comment 14 err 2007-05-18 04:52:51 UTC
Wondering about the impact of not having a focusBias property as described, I
considered jvi's old tricks while using the allowedSearchDirection algorithms.
The command mode tricks are:
1- run matchBraceAction, if no movement then {beep; return}
2- back up the caret

This generally works, but depends on *synchronous* behavior. Not the case here.

Also there is an ambigous situation. Consider "...(|)..." using Forward
Preferences, the caret would not move when matchBraceAction, so there's no way
to detect if the match was successful.

Identified a bug in the algorithms, see code fragment below. With "Forward
Preference" and carete as shown, do Ctrl-[ and nothing happens. The same is true
for other allowedSearchDirection.

    void s|tuff() {
        moreStuff(5);
        main.main(new String[1]);
    }
Comment 15 Miloslav Metelka 2007-05-18 15:54:17 UTC
Yes, I'm also wondering what the caret.getDot() should return in jvi command
mode. The cursor cannot go before the first char or beyond the last char on a
line. It could possibly be modeled like standing before or after the character
designated by the rectangle. Should either one of these two preferred for some
reason?
Comment 16 err 2007-05-18 17:55:56 UTC
> wondering what the caret.getDot() should return in jvi command mode
Consider that Document.getText(caret.getDot(), 1) is the character "under" the
block cursor, so the swing model is ok for jvi command mode.

Editors like netbeans that have some issues with caret.getDot(). Consider this
code from ExtKit.MatchBraceAction (rev 1.71 before recent patch)
    int[] matchBlk = sup.findMatchingBlock(dotPos - 1, false);
Notice the "- 1". In rev 1.72 the AFTER/BEFORE patch worked this out.

So editors get the discrepancy built in, creating problems for jvi reuse of
existing actions/behavior. I believe the discrepancy also complicates the
allowedSearchDirection algorithms, e.g. first check the character before the
cursor, then start looking forward.

If the algorithms have a concept of bias for the "important" character, it might
actually simplify the match origin search algorithms.
Comment 17 Vitezslav Stejskal 2007-05-20 04:18:37 UTC
> AFTER/BEFORE indicate which character is important or "focused" for
> the purpose of brace highlighting and goto-match.

I understand this.

> So AFTER/BEFORE affect the caret positioning in a matchBraceAction.
> In input mode, the character BEFORE the caret is important; this is
> the same as netbeans.

> So AFTER/BEFORE is orthogonal to the allowedSearchDirection property.

Ok, they are, when talking about matchBraceAction. But they are pretty much the
same when talking about findOrigin algorithm (they are just some parameters
affecting the way how we search for the origin).

That was probably my biggest confusion. So, we need to specify position bias for
matchBraceAction and make it navigate either in front of the matching area or
behind it. Is that what you are saying? Another property independent on
allowedSearchDirection influencing only the action. I guess this should be used
in Nb for overwrite mode, which uses block cursor too.

Re. BadLocationException: I have no strong preference about having it in the
SPI. I added it merely because every BracesMatcher implementation will need to
access document functions that throw BLE and they all will need to try-catch the
exception and log it, if it's not possible to rethrow it. Semantically when
there is no origin the search does not fail, it succedes returning null. A
failure means that the search could not finish, because of some problems/errors.
Anyway, the javadoc is poor.

Re. InterruptedException: I know this is confusing, but in general interrupting
a thread can result in either its status being set to 'interrupted' or
InterruptedException being thrown and the status being cleared. What case is
used depends on what the thread is doing when it's interrupted. If it's waiting
in wait(), join(), sleep() or similar the IE is thrown. And it *must* be
rethrown to the bracesmatching infra. If the thread is working its status is set
to interrupted, in which case it does not really matter if the BracesMatcher
simply quits or throws its on IE. So, in the example returning null is fine,
faster than throwing IE, which would be ok too.
Comment 18 Vitezslav Stejskal 2007-05-20 04:49:47 UTC
> If the algorithms have a concept of bias for the "important" character,
> it might actually simplify the match origin search algorithms.

I see, if had the position/focus/caret/whatever bias property and used it for
both the search origin algorithm and the matchBraceAction we could get rid of
the 'backward/forward-preferred' values of the allowedSearchDirection property.
It sound like a cunning plan, I'll try to implement it.
Comment 19 Vitezslav Stejskal 2007-05-20 08:31:09 UTC
I add the 'nbeditor-bracesMatching-caretBias' property and adjusted the control
panel accordingly. Can you have a look and see if it's working the way as you
would expect in jVi. The new property simplified things a little bit and it also
covers the navigation usecase you mentioned, which is great. Thanks for all the
feedback you have given us so far.

Milo, how can I check what mode the editor is in - normal vs. overwrite? I think
we should automatically switch the caretBias depending on the mode if the
property is not specified. Thanks.
Comment 20 Miloslav Metelka 2007-05-21 16:39:53 UTC
A sort of weird "editorUI.getProperty(EditorUI.OVERWRITE_MODE_PROPERTY)" should
return Boolean saying whether the editor is in overwrite mode or regular insert
mode.

Caret's impls should honor RTL (right-to-left) text by using a bias e.g.
DefaultCaret does but our BaseCaret does not. I didn't study the RTL issues much
but IMHO we should use "bias" to express command/insert modes to prevent
confusion with RTL related things. 
Just to clarify: For jvi there should probably be an extra "commandMode" boolean
flag because the overwriteMode flag makes sense in the typing mode (when
pressing "R" in command mode the editor would be in the overwrite typing mode
oM=true; when pressing e.g. "i" it would be insert typing mode oM=false).
BTW Vim uses a square highlight for normal mode; pipe for insert typing mode and
underline for overwrite typing mode.
Comment 21 err 2007-05-21 22:59:02 UTC
"AllowDir: Forward, CaretBias: Forward" is exactly what vim findMatch does,
and it has the advantage that you don't have to be right next to the character
for the highlight to appear. Very nice, thanks.

The change in AllowDir is different from expected. I thought there would
be "Both", "Forward-preferred", "Backward-preferred" options. And you could
get the Forward behavior by restricting the backward looking as in
    MaxBack: 0, MaxForward: 256, AllowDir: Forward, CaretBias: Forward
I would let the jvi user have forward-preferred, as option or even default,
since it seems superior behavior.  Made a small change to play with it, its
attached as a patch to show you.

Things didn't work exactly as expected regards to MaxBack, MaxForward.
After playing with the patch with MaxBack 200, MaxForw 200, I tried the
settings: MaxBack: 0, MaxForward: 200, AllowDir: Forward, CaretBias: Forward
to verify that the original AllowDir "Forward" worked as before the patch.
I couldn't set it to zero due to a code restriction, then I tried 1.
With MaxBack at 1, consider the code fagment with cursor right after the "}":
    }| // zzz
This did *not* highlight the "}" (set to 2 it was highlighted). Still with
MaxBack at 1, changed caret-bias to back and then it did highlight. I'm not
necessarily saying this interaction between MaxBack and caret-bias is a bug,
but it was unexpected.

BTW, the patch also contains a tweak to ControlPanel so that the
AllowSearchDirection property can be set to forward.
Comment 22 err 2007-05-21 23:13:02 UTC
> Just to clarify: For jvi ...
Milo, I dont' understand what you're addressing. jVi currently has "R" replace
mode and uses the three cursor shapes you mention as appropriate (BTW, <BS> in
replace mode DTRT). It also shows a half high block cursor for operator
entered and a fat pipe cursor for visual mode when 'sel' is exclusive, both as
in vim.
Comment 23 err 2007-05-21 23:17:14 UTC
Created attachment 42622 [details]
Proposed alternate behavior for "Forward" allowDir
Comment 24 Miloslav Metelka 2007-05-22 13:05:51 UTC
Apologies for confusion the latter part was not related to brace matching
directly. I'm trying to think how the BaseCaret etc. could be reused for jvi and
possibly other editor emulations. There could be the two mode flags:
overwriteMode and commandMode that the caret would check and modify its
appearance accordingly. BTW is jvi impl have its own caret impl?

Regarding caret bias naming: will it appear publicly in the SPI e.g.
MatcherContext.getCaretBias() or not? I see now it's only in the impl and the
only public place where it appears is the
component.putClientProperty("nbeditor-bracesMatching-caretBias") which is fine
since it's clear that it's unrelated to the bias used in caret methods (e.g.
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4252044 )

Not a strong opinion but regarding InterruptedException in BracesMatcher: IMHO
there might be problems if the impl will be calling some other framework that
would not be prepared well for the cases of the thread interruption. IIRC when
thread interrupting possibility was introduced in RequestProcessor's tasks the
impl with interrupting was made a default. However the other day after the
commit it had to be reverted because the interruptions were coming in the middle
of reads and writes to the MDR (metadata repository) files which was a total
disaster causing the whole MDR to fail and report corruptions instantly.
Alternatively we could have BracesMatcher.cancel() that could still do
Thread.interrupt() if it would be safe to do so. For many cases it could suffice
to just have "boolean MatcherContext.isCancelled()" which would be checked e.g.
after each scanned token (assuming the lexical scanning for matching brace).
Maybe we could make the BracesMatcher an abstract class with cancel() doing
Thread.interrupt() by default but the impls could override it if necessary.
Comment 25 err 2007-05-22 16:15:00 UTC
>BTW is jvi impl have its own caret impl?
jvi extends NB's ExtCaret for painCustomCaret, for the 5 cursors it currently
implements. It also overrides several methods so it can tweak setDot to keep the
caret off the newline during a mouse click.
Comment 26 Vitezslav Stejskal 2007-05-23 03:58:46 UTC
Re. thread interruptions: Well, we don't have MDR anymore. Does lexer have the
same problem? Or new java infrastructure? I really like the way how it is now
and the fact that the task interruption does not clutter the SPI, but if you
think it's a real problem I'll have to redesign it of course.
Comment 27 Vitezslav Stejskal 2007-05-23 04:12:12 UTC
Ernie, thanks for the patch.

> I thought there would be "Both", "Forward-preferred", "Backward-preferred"
> options. And you could get the Forward behavior by restricting
> the backward looking as in
>    MaxBack: 0, MaxForward: 256, AllowDir: Forward, CaretBias: Forward

If we have 'Forward-preferred', 'Backward-preferred' and maxBack/Forward, then
the caretBias is good only for matchBraceAction, right? In which case I'd like
to call it something else.

I really want to make sure that the meaning of all those properties is clear and
*simple*. I don't want to have any complex relations between them.
Comment 28 err 2007-05-23 06:26:22 UTC
> I really want to make sure that the meaning of all those properties is
> clear and *simple*. I don't want to have any complex relations between
> them.

Yes. I think that's the most important issue.

> If we have 'Forward-preferred', 'Backward-preferred' and maxBack/Forward,
> then the caretBias is good only for matchBraceAction, right?

It can be defined either way, what's simplest to understand? I've been
assuming that if bias applies to one then it would be simplest to
understand if it applied to both.

Here's a case to examine
    }| // zzz

METHOD 1:
First assume that caretBias always applies.  If backbias, then no matter
the algorithm, the '}' and its match are highlighted. So understanding the
situations where caretBias always applies depends on using the forw/back
count from the ground 0 "important" character.

METHOD 2:
On the other hand, if careteBias (or whatever it would be called) only
applies to which side of the match the caret ends up on when you do a goto
match, there is no "important" character; ground 0 is the point between
characters.

Finally, if the capabilities of the two conventions are the same, then I
don't think it matters which method is selected because very few will
actually program the properties directly.  Do the two methods give the same
capability? I think they're a litte different, but that it doesn't really
matter.

METHOD 2 is probably simpler to understand, since when working with
documents it's all relative to the caret position; and caretBias is only
for tweaking the caret at the end of the match (Just talked myself out of
my favorite. I think I was trying to build "intput/command mode" into the
caret, I think Milo was hinting at this) This should simplify the code, the
only time bias matters is for final tweaking when everything's done.

I'm assuming that in either case, when you execute the goto match, you end
up at the farthest highlighted brace from the caret.

> In which case I'd like to call it [the caretBias] something else.

Cool.
Comment 29 err 2007-05-23 06:36:23 UTC
Considering: ant -Dtryme.args="-J-Dnbeditor-no-HighlightBraceLayer=true" tryme
When I was playing around and wanted to run this under the debuger, I modify
nbbuild/build.xml: <target name="tryme-setup-debug-args" if="debug.port" >
add: name="tryme.debug.args" value="-J-Dnbeditor-no-HighlightBraceLayer=true
and used the tryme-debug target. Is there a better way than modifying build.xml?

If I want to run jvi with this mix, can you point me to something so I can see
how to set it up?
Comment 30 Vitezslav Stejskal 2007-05-23 07:07:03 UTC
I think -Dtryme.args should also work when debugging. Did you try it?

> If I want to run jvi with this mix, can you point me to something
> so I can see how to set it up?

Sorry, I don't understand. What is the problem? Just build the
EditorBracesMatching module and your jVi support and run Nb with the switch...
Comment 31 err 2007-05-23 07:27:33 UTC
Keep in mind, I'm actually a novice NB user. When you say
> I think -Dtryme.args should also work when debugging. Did you try it?
where would I put it?

To debug jVi, I set the jvi module-suite as main project and debug.

Ohhhhh, maybe I get it. When I build the BracesMatcher from contrib, it probably
installs itself into the platform. So when I debug jvi on that platform it is
there, what I need to know is where to put the -Dtryme.args. Somewhere in jvi's
module-suite settings?
Comment 32 Vitezslav Stejskal 2007-05-24 01:22:27 UTC
Oh, sorry, I didn't realize that your using a suite. Of course it's obvious now.
Yes, when you build EditorBracesMatcher it should install itself in the platform
in <cvs-source-tree>/nbbuild/netbeans. In your suite go to 'File' tab and look
in the <jVi-suite>/nbproject/private/private.properties file (create it if it
does not exist, it's just an ordinary text file). In this file add:

run.args.extra=-J-Dnbeditor-no-HighlightBraceLayer=true

The property should be used for both run/debug of your suite.

Also, <cvs-source-tree>/nbbuild/netbeans/harness/README is a nice reading ;-).
There is a lot of stuff that you can set in the properties files, which does not
have any GUI in project properties yet. Sidenote: the contents of
nbproject/private should be ignored by CVS, it's your private properties. For
shared properties use nbproject/project.properties.
Comment 33 err 2007-05-24 04:27:06 UTC
>in the <jVi-suite>/nbproject/private/private.properties ... file add ... -D...
Thanks, that did the trick.

I'm not able to access the MasterMatcher for the property names. I'm assuming
that this has something to do with the setup and is not a real issue.

Will BracesMatchHighlighting.propertyChange also check for algorithm parameters
to do a refresh?
Comment 34 err 2007-05-24 05:00:16 UTC
My analysis of the two methods was flawed in the conclusion
>Do the two methods give the same capability? I think they're a 
>little different, but that it doesn't really matter.
That statement is correct except for the "[the difference] doesn't really matter".
(I spent too much time convincing myself that forward-preffered could be turned
into forward by tweaking the maxBack/Forw.)

It turns out that the way it works now, with cursorBias affecting the highlight
is  needed to get the preferred behavior. This is forward-preferred, basically
both but without the dead zone. (I think most people will prefer this, why do
you need a dead zone? BTW, overall it feels real good in the editor.)

A case is
     foo(bar(a)|)
If in input mode the inner pair of parens should be highlighted, but in command
mode the outer pair.

I can't see a way to do this without having the bias affect the highlight as
well as the gotoMatch. (I was halucinating, sorry.)

So METHOD 1 works best for jvi. If you'd like to examine other cases let me know.
Comment 35 Vitezslav Stejskal 2007-05-24 05:15:47 UTC
> Will BracesMatchHighlighting.propertyChange also check for algorithm parameters
> to do a refresh?

Yes, it will.
Comment 36 Vitezslav Stejskal 2007-05-24 08:42:45 UTC
> I'm not able to access the MasterMatcher for the property names. I'm
> assumingthat this has something to do with the setup and is not a real issue.

You won't be able to access it, the class is not part of the API. If fact the
module has no API classes at all, its only API will be the properties documented
in the architecture document.
Comment 37 Vitezslav Stejskal 2007-05-25 04:13:29 UTC
Thinking more about those two methods you described, I agree that M1 (using
caretBias consistently for both highlighting and navigation) works better. I
made up my mind about what the parameters interpretation should be and done some
changes to the module. I'm quite happy about it now, because it looks
understandable and covers the usecases we have. Here is what we have now:

caretBias: determines the 'important' character, it's either 'backward' or
'forward', so the important character is either in front of the caret or behind it

searchDirection: determines the algorithm how we search for the origin, two
values are currently supported 'forward-preferred' and 'backward-preferred'.
Basically it means the module looks in the 'preferred' direction and if nothing
is found it tries the opposite direction

maxBackwardLookahead, maxForwardLookahead: the maximum distance measured from
the position of the caret (!) how far the search can be done. Possible values
are 0-256, but the search never leaves the line with the caret.

The important character as determined by caretBias is always checked first, no
matter what direction we are searching in and no matter what the max lookahead
value for this direction is.

Here is a few interesting usecases:

A. Look only at the important character: MFL = MBL = 0, searchDirection does not
matter, changing caretBias determines whether the detection happens before or
after the caret

B. Look 1 character at each side of the caret: MFL = MBL = 1, searchDirection
does not matter, caretBias determines the side to look at first. For caretBias =
Backward this is the current state in a trunk build with the old infrastructure.

C. jVi normal mode: MBL = 0, MFL = 256, searchDirection = F, caretBias = B

D. jVi command mode MBL = 0, MFL = 256, searchDirection = F, caretBias = F

I dropped the 'dead zone' usecase. It is implementable, but does not fit to the
scheme with the others. I only included it because somebody mentioned it in
#95126. I am not sure if we ever decide to use it, but if we do we will be able
to add it.

For Nb6 I'd like to make the editor alternate between B and A(cB = F) for normal
and overwrite modes respectively.

I hope I haven't missed or misuderstood something. Is the above good? Thanks.
Comment 38 err 2007-05-25 19:34:25 UTC
A. default NB overwrite: MBL = 0, MFL = 0, SD = ?, CB = F
B. default NB insert:    MBL = 1, MFL = 1, SD = ?, CB = B

C. default jVi insert:  MBL = 0, MFL = 256, SD = F, CB = B
D. default jVi command: MBL = 0, MFL = 256, SD = F, CB = F

E. jVi option insert:  MBL = 256, MFL = 256, SD = F, CB = B
F. jVi option command: MBL = 256, MFL = 256, SD = F, CB = F

jVi will offer 'F' as an option, if for no other reason than that I want to
use it. Also with E and/or maybe B as an option, or maybe offer "lookahead"
and "lookbehind" boolean options, or an expert option to tweak the numbers.

(Curiosity, in case B., is it the same if MBL == 0?)

> For Nb6 I'd like to make the editor alternate between B and A(cB = F) for
> normal and overwrite modes respectively.

I suspect if NB doesn't have some options other than A,B, there will be
disappointed users.

This feels real good running it in jvi. If Class.forName("...BracesMatcher)
jvi uses this.

Go for it!
Comment 39 Vitezslav Stejskal 2007-05-28 00:44:18 UTC
Wonderful, I like E, F, we should have something like that in Nb too. I'll try
to push for some way to customize this in Tools-Options, but we are getting
close to feature freeze now.

Re. option B with  MBL == 0: yes it is the same. Lookaheads 0, 1 in the same
direction as CB behave the same.
Comment 40 err 2007-05-29 03:45:47 UTC
In the docs under "Use case 3. - Different search scenarios" there's a type. For
scenario D. "MFL = 0," should be "MFL = 256,".

Question. When jVi attaches to a document it stashes the caret, and restores it
 if jvi is disabled. Should it save/restore the matching policy paramaters as well?

Curiosity, what are the next steps to get this incorporated?
Comment 41 err 2007-05-29 03:49:10 UTC
Also, in the section "Controlling the parameters" for ...-searchDirection it
shows  "backward-preferred  and forward-preferred" as values, but the current
code does not have the "-preferred".
Comment 42 err 2007-05-29 04:57:10 UTC
With MBL = 256, MFL = 256, SD = F, CB = F, with the following
    |void stuff() {
the match brace action does not move the cursor. The "()" is highlighted.
Comment 43 Vitezslav Stejskal 2007-05-29 06:55:54 UTC
Thanks for proof reading and testing. It should all be fixed now. The next step
is api review and then if/when the spi is approved I'll move the module under
the editor folder and add it to the standard build (ide cluster).
Comment 44 Vitezslav Stejskal 2007-05-29 07:34:08 UTC
I'd like to ask for review of the braces matching SPI. The SPI and the
infrastructure behind it is currently in the contrib/EditorBracesMatching
module, which can be installed in a trunk build and used for testing the
existing matchers and the infrastructure itself. The old braces matching and its
highlighting layer can be turned off by setting
-J-Dnbeditor-no-HighlightBraceLayer=true.

The module itself implements several matchers, the most important ones are the
default and the legacy matchers. The legacy matcher is used for documents that
still implement braces matching by subclassing
org.netbeans.editor.ExtSyntaxSupport. The default matcher is used when there is
no other matcher available. Also as a proof of concept I am working on a proper
matcher for java.

The discussion in this issue is rather long and probably doesn't have to be
closely followed, because all its outcomes are summarized in the SPI
documentation. I'll attach the documentation to this issue for easier viewing.

I am asking for the fast track review, because I think the SPI is reasonably
simple and non-controversial, but if people think that this needs more thorough
discussion we will switch to normal review.
Comment 45 Vitezslav Stejskal 2007-05-29 07:35:15 UTC
Created attachment 42875 [details]
The Braces Matching SPI documentation
Comment 46 Vitezslav Stejskal 2007-05-29 07:41:30 UTC
Readily browsable URL for the documentation:
jar:http://www.netbeans.org/nonav/issues/showattachment.cgi/42875/org-netbeans-modules-editor-bracesmatching.zip!/index.html
Comment 47 Vitezslav Stejskal 2007-06-04 11:27:39 UTC
If there are no objections I'll put this in the standard build later this week.
Comment 48 Vitezslav Stejskal 2007-06-07 10:10:57 UTC
In trunk now, I'll add the commit log as an attachement.
Comment 49 Vitezslav Stejskal 2007-06-07 10:11:57 UTC
Created attachment 43360 [details]
Commit log from the CVS server
Comment 50 Vitezslav Stejskal 2007-06-07 10:12:51 UTC
Done.