bug#50236: 27.2; electric-pair-mode is inconvenient in comint

bug#50236: 27.2; electric-pair-mode is inconvenient in comint

[Augusto Stoffel]

In comint buffers, electric pair mode should only look at the current
input region to decide whether to skip over a closing bracket or add a
new one.  Otherwise, it gets confused about mismatched delimiters in
previous inputs or outputs.

To give an example, if I enter, in a fresh shell

    X='('

at the first prompt, and then type “())” in the second prompt, I get
the following sequence of states (where | indicates the point)

    |
    (|)
    ()|)
    ())|

where I would instead expect the obvious:

    |
    (|)
    ()|
    ())|

bug#50236: 27.2; electric-pair-mode is inconvenient in comint

[Augusto Stoffel]

The following quick fix works for me:

    (defun electric-pair-skip-in-field (char)
      (save-restriction
        (narrow-to-region (field-beginning) (field-end))
        (electric-pair-default-skip-self char)))

    (add-hook 'comint-mode-hook (lambda () (setq-local electric-pair-skip-self
                                                       'electric-pair-skip-in-field)))

Perhaps `electric-pair-default-skip-self' should always narrow to the
current field?

There are a few more situations where electric-pair-mode looks to far;
for instance, when inside an org src block, mismatched parenthesis
outside the block shouldn't matter.  So maybe an even more general
solution is in order.

On Sat, 28 Aug 2021 at 12:17, Augusto Stoffel <arstoffel@gmail.com> wrote:

> In comint buffers, electric pair mode should only look at the current
> input region to decide whether to skip over a closing bracket or add a
> new one.  Otherwise, it gets confused about mismatched delimiters in
> previous inputs or outputs.
>
> To give an example, if I enter, in a fresh shell
>
>     X='('
>
> at the first prompt, and then type “())” in the second prompt, I get
> the following sequence of states (where | indicates the point)
>
>     |
>     (|)
>     ()|)
>     ())|
>
> where I would instead expect the obvious:
>
>     |
>     (|)
>     ()|
>     ())|

bug#50236: 27.2; electric-pair-mode is inconvenient in comint

[Lars Ingebrigtsen]

Augusto Stoffel <arstoffel@gmail.com> writes:

> The following quick fix works for me:
>
>     (defun electric-pair-skip-in-field (char)
>       (save-restriction
>         (narrow-to-region (field-beginning) (field-end))
>         (electric-pair-default-skip-self char)))
>
>     (add-hook 'comint-mode-hook (lambda () (setq-local electric-pair-skip-self
>                                                        'electric-pair-skip-in-field)))
>
> Perhaps `electric-pair-default-skip-self' should always narrow to the
> current field?

That would make sense in this case...  I'm trying to think of instances
where it wouldn't make sense, and I can't think of any.

So perhaps we should make this change in general?

Anybody have any opinions here?

bug#50236: 27.2; electric-pair-mode is inconvenient in comint

[Augusto Stoffel]

On Mon, 22 Aug 2022 at 17:37, Lars Ingebrigtsen <larsi@gnus.org> wrote:

>
> Augusto Stoffel <arstoffel@gmail.com> writes:
>
>> The following quick fix works for me:
>>
>>     (defun electric-pair-skip-in-field (char)
>>       (save-restriction
>>         (narrow-to-region (field-beginning) (field-end))
>>         (electric-pair-default-skip-self char)))
>>
>>     (add-hook 'comint-mode-hook (lambda () (setq-local electric-pair-skip-self
>>                                                        'electric-pair-skip-in-field)))
>>
>> Perhaps `electric-pair-default-skip-self' should always narrow to the
>> current field?
>
> That would make sense in this case...  I'm trying to think of instances
> where it wouldn't make sense, and I can't think of any.

There's a second question of relevance here: would this change help
solving similar bugs in other modes?  Consider for instance an Org file
like this


    (

    #+begin_src
      f(<type close parens here>)
    #+end_src

or a Markdown file like this

    (

    ```
      f(<type close parens here>)
    ```

Of course each of these modes could define their own
electric-pair-skip-self, but ideally a general mechanism to deal with
this situation should we provided.

So I guess my question here is: does it make sense for a major mode with
a notion of "code blocks" set field properties as part of the
font-locking?  Or is there any reason not to mix up fields with
font-locking?

bug#50236: 27.2; electric-pair-mode is inconvenient in comint

[Lars Ingebrigtsen]

Augusto Stoffel <arstoffel@gmail.com> writes:

>> That would make sense in this case...  I'm trying to think of instances
>> where it wouldn't make sense, and I can't think of any.

[...]

> So I guess my question here is: does it make sense for a major mode with
> a notion of "code blocks" set field properties as part of the
> font-locking?  Or is there any reason not to mix up fields with
> font-locking?

Hm, that's an interesting question.  I think that, basically, the only
usage for the "field" thing is to divide a line up into bits so that
`C-a' takes you to the start of the field instead of the start of the
line.  Extending the "field" thing to mark larger blocks is might well
make sense.

Anyway, this reminds me of a performance problem we have when making
commands field sensitive: It's generally kinda slow.

It's no problem in the `C-a' case -- we're limited to the current line,
so our search for field properties is very short.  I was making some
other command field sensitive (I forget which), but had to abandon it,
because it was too slow.  The problem is, generally, that when you're
not in a field, you want to find the end the previous field and delimit
the command to that region.

However, the previous field may be anywhere, so the searches for the
field text property goes back to point-min.  And that's just unworkably
slow for functions that trigger a lot -- and I think that this may be
the case for electric-pair-mode, too.

I mean -- we could delimit electric-pair-mode to the current field, if
there is one, but we can't do the same if we're not in a field.  So if
you have

---
*Here's a field with (*


Here we, much later in the buffer, are outside of a field and we type )
---

we can't (for these performance reasons) just use a `narrow-to-field'
first when checking whether that ) matches that other (.  Once we have
the pairs, we could check whether both are in the same field, though.

bug#50236: 27.2; electric-pair-mode is inconvenient in comint

[Augusto Stoffel]

On Tue, 23 Aug 2022 at 11:53, Lars Ingebrigtsen <larsi@gnus.org> wrote:

>
> Augusto Stoffel <arstoffel@gmail.com> writes:
>
>>> That would make sense in this case...  I'm trying to think of instances
>>> where it wouldn't make sense, and I can't think of any.
>
> [...]
>
>> So I guess my question here is: does it make sense for a major mode with
>> a notion of "code blocks" set field properties as part of the
>> font-locking?  Or is there any reason not to mix up fields with
>> font-locking?
>
> Hm, that's an interesting question.  I think that, basically, the only
> usage for the "field" thing is to divide a line up into bits so that
> `C-a' takes you to the start of the field instead of the start of the
> line.  Extending the "field" thing to mark larger blocks is might well
> make sense.
>
> Anyway, this reminds me of a performance problem we have when making
> commands field sensitive: It's generally kinda slow.
>
> It's no problem in the `C-a' case -- we're limited to the current line,
> so our search for field properties is very short.  I was making some
> other command field sensitive (I forget which), but had to abandon it,
> because it was too slow.  The problem is, generally, that when you're
> not in a field, you want to find the end the previous field and delimit
> the command to that region.
>
> However, the previous field may be anywhere, so the searches for the
> field text property goes back to point-min.  And that's just unworkably
> slow for functions that trigger a lot -- and I think that this may be
> the case for electric-pair-mode, too.
>
> I mean -- we could delimit electric-pair-mode to the current field, if
> there is one, but we can't do the same if we're not in a field.

This makes sense.  However, in comint the "input" field has no field
property; only the output is labeled as such.  So the suggestion to do
something special if inside a field wouldn't solve the original problem
described in the bug.

So the conclusion seems to be that a comint-specific
electric-pair-skip-self function is needed (namely, one that narrows to
the current field provided the current field property is nil, to avoid
those performance issues.)

bug#50236: 27.2; electric-pair-mode is inconvenient in comint

[Lars Ingebrigtsen]

Augusto Stoffel <arstoffel@gmail.com> writes:

> This makes sense.  However, in comint the "input" field has no field
> property; only the output is labeled as such.  So the suggestion to do
> something special if inside a field wouldn't solve the original problem
> described in the bug.

Ah, right.

> So the conclusion seems to be that a comint-specific
> electric-pair-skip-self function is needed (namely, one that narrows to
> the current field provided the current field property is nil, to avoid
> those performance issues.)

The other possible solution (that I mentioned, but didn't expand on) is
that we could just fix this in electric-pair without relying on
narrow-to-field.  That is, once electric-pair has found the matching
pair, we just look at the region between the two chars and see whether
they are part of the same field.  That should be reasonably fast, since
electric-pair already limits the range it's willing to search for a
pair.