Aggressively narrow some external functions

[akater]

This is a draft. We aim to

• narrow the scope of some external functions to the current chunk
• allow some “inner” major modes to initialize without errors

As written, this mitigates #255 w.r.t. paredit for me. We add global advices on loading polymode. How about we provide a custom variable to specify which functions to advice?

I did not research the smartparens part as I don't use it. I'll dig deeper if the overall idea gets approved.

[vspinu]

Thanks!

All compatibility stuff should go into polymode-compat.el as much a possible.

[vspinu]

I don't understand why this should be happening when the mode is installed. It's a global advice, so it could be done just fine like other advices in polymode-compat.el or not?

I would actually very much prefer it to be a well documented stand alone advice and not a programatically installed one somewhere in the core code. This way people could easier find out who is advising and why.

[akater]

I see that you alerady have this mechanism. This begs several questions. In order of decreasing importance,

1. Suppose we

(pm-around-advice 'paredit-mode #'pm-execute-narrowed-to-span)

and initialize an Org buffer with lisp blocks, and with paredit-mode being loaded in lisp-mode-hook.

In what case then will paredit-mode get enabled? Do paren checks need to get passed in each standalone inner lisp-mode span? (Such ambiguity was the reason why I preferred to enable paredit-mode unconditionally.)

This better be in the documentation; I couldn't find the answer. In fact, the whole polymode-compat facility better be mentioned there.

2. Why is *span* not defined buffer-local?

3. Why does its name violate Elisp conventions? Why not pm--span or pm--narrowing-span?

4. When I evaluate *span* in an Org block in poly-org, i still see nil value which makes it unclear how does it work exactly. Could something be written in documentation to highlight that? Docstring for pm-execute-narrowed-to-span says

*span* in pm-map-over-spans has precedence over span at point.

which does not help.

5. Why wasn't paredit-mode advised aready in the last couple of years? (Rhethorical question but since polymode-compat was not mentioned in Protect smartparens, paredit, check-parens (others?) to operate within current chunk #255, I presumed there's nothing even to start with and I ended up wasting more time than needed.)

[vspinu]

Sorry for late response. I have missed your reply somehow.

(pm-around-advice 'paredit-mode #'pm-execute-narrowed-to-span)

I don't use paredit but I think this is not what you want. As the name suggests pm-execute-narrowed-to-span should be added as an advise to the actual functions which need to operate within a span.

Paredit's situation might be a bit more complex if it has to parse the entire buffer and does assume homogenuous syntax. If it simply relies on parse-ppss then it should work out of the box if it's activated globally.

Why is span not defined buffer-local?
Why does its name violate Elisp conventions? Why not pm--span or pm--narrowing-span?

Because it's a dynamically bound variable available only within the mapping functions. It does't make sense to bind it globally or locally in the buffer. I have clarified the docs of those compact functions. *span* is bound in the pm-map-over-span.

Why wasn't paredit-mode advised aready in the last couple of years?

No one reported a simple and clear reproducible issue. I am not a user of paredit mode so I cannot see the issue by myself either.

[vspinu]

I am looking at this again after some years, and it's still not making a lot of sense to me.

I think a much smaller and more readable incision could be done here without these extra constructs. To summarize the comments, I think all what's needed is to:

1. Advice whatever functions need to be narrowed with pm-execute-narrowed-to-span
2. Advice worker functions to do what's needed conditionally on if it's in a polymode buffer (ex. paredit-override-check-parens-function)
3. (most likely unnecessary) Advice mode functions to behave differently in the polymode buffers (conditional on polymode-mode being t)

[vspinu]

polymode-compat is very dirty, it should not be included in the core.

[vspinu]

This just can be done by advising the mode function. I don't see the need for such a hack in the polymode proper.

[vspinu]

I don't understand all these changes. These are essentially internals of smartparens and paredit. Also the comments do explicitly say it's either "wrong" or "unsure about". So it's clearly not "production" ready.

[vspinu]

If narrowing to current span is all what is needed, then one could just do:

(pm-around-advice 'some-function #'pm-execute-narrowed-to-span)

[akater]

Thank you. Yes, almost certainly, significant rewrites are needed. I submitted what seemed to be working for me just because it's a better start than an issue without any patch. Hopefully, I'll rewrite it soon.