parse-partial-sexp to stop at any depth change (and Re: Fontifying outside of region passed to registered JIT lock function)

parse-partial-sexp to stop at any depth change (and Re: Fontifying outside of region passed to registered JIT lock function)

[Barry OReilly]

[-- Attachment #1: Type: text/plain, Size: 2120 bytes --]

> insert a "display only" close-paren (with suitable coloring).

There's probably less potential to cause user confusion if it colors
existing parens rather than color parens that are not really a part of
the file content.

> 1- do add the face on the text before the beginning of the region.
>    The problem there, is to make sure this face is kept up-to-date
>    later on (e.g. removed if you add revert to the previous
>    indentation). For that you'll want to add a jit-lock-multiline
>    text-property.

I found the right colors seem to display at the right times if I:

  • For initial fontification: do add the face outside the region
  • For fontification after changes: rely on a new
    jit-lock-after-change-extend-region-functions hook to expand the
    region

Since that is working, I began investigating the performance.

One issue is my usage of parse-partial-sexp. This function's interface
doesn't make it easy to stop if there's a change of depth either up or
down, so for code simplicity I went one char at a time:

  (parse-partial-sexp (point)
                      (1+ (point))
                      nil
                      nil
                      parse-state
                      nil)

To see if this was responsible for performance degradation, I hacked
the C implementation to allow a caller to specify "stop at a change of
depth either up or down" and used it. The performance improvement was
huge.

Now, I could write code that would do two sexp parses concurrently,
searching for a change of depth up and a change of depth down, saving
the result of the furthest parse so as to not create an O(n*n)
algorithm. Rather than complect my code in this way, what would you
think of extending the parse-partial-sexp in one of two ways:

  • If TARGETDEPTH is t, stop when any change of depth occurs
  • If TARGETDEPTH is a vector of integer depths, stop when any of
    those depths is reached.

I figure I can't be the only one who would want to have
parse-partial-sexp stop at any change of depth. May I prepare a patch
to provide this?

[-- Attachment #2: Type: text/html, Size: 2327 bytes --]

Re: parse-partial-sexp to stop at any depth change (and Re: Fontifying outside of region passed to registered JIT lock function)

[Stefan Monnier]

>> insert a "display only" close-paren (with suitable coloring).
> There's probably less potential to cause user confusion if it colors
> existing parens rather than color parens that are not really a part of
> the file content.

But if that existing paren is before window-start, the user won't see
it.  Using fake close-parens might not be the best choice, but I think
the idea of having some visual indicator saying "not enough
whitespace here or missing close parens on previous lines" between BOL
and the first non-whitespace char can be workable.  This visual
indicator can be to just highlight the indentation space, but as you say
there might not be any indentation space, so inserting a display-only
char can be a good solution.  Now which char to use is up to you.

> One issue is my usage of parse-partial-sexp. This function's interface
> doesn't make it easy to stop if there's a change of depth either up or
> down, so for code simplicity I went one char at a time:

>   (parse-partial-sexp (point)
>                       (1+ (point))
>                       nil
>                       nil
>                       parse-state
>                       nil)

Yuck!  Look at (nth 9 (syntax-ppss)): it's a list of the positions of
the open-parens that aren't closed yet.


        Stefan