Title: Suggested Advice Usage

[tvraman]

Description: In line with your docstring that points out that the current advice is a bit of a kludge, here is one way to more clearly reflect the underlying intent of the advice --- see code below: it uses an around advice using the features of the original advice.el package.

It also avoids the need to call keyboard-quit at the end of the advice. This latter is particularly annoying at present as I try to speech-enable the crossword package for emacspeak -- see https://github.com/tvraman/emacspeak

;;; If we only want this for interactive calls, we can further tighten it with a check on called-interactively-p

(defadvice self-insert-command (around crossword pre act comp) "Advice for handling insertions in the 'Crossword grid' buffer. It controls data-entry, fontification, advances POINT to the next grid position, and updates the puzzle's completion statistics. it avoids the original function being called by never calling ad-do-it. " (when (and (eq major-mode 'crossword-mode) (get-text-property (point) 'answer) (not (get-text-property (point) 'solved))) (crossword--insert-char) (if crossword-auto-nav-only-within-clue (crossword--next-square-in-clue 'wrap) (crossword--next-logical-square))))

[Boruch-Baum]

On 2021-01-21 10:53, T. V. Raman wrote:

Description: In line with your docstring that points out that the current advice is a bit of a kludge, here is one way to more clearly reflect the underlying intent of the advice --- see code below: it uses an around advice using the features of the original advice.el package.

How have you tested this and got it to work? I ask because my original implementation was an around advice and it would reliably immediately crash my emacs (circa versions 26/25) with some kind of recursion error IIRC. As in your code, my original never called ad-do-it.

It also avoids the need to call keyboard-quit at the end of the advice. This latter is particularly annoying at present as I try to speech-enable the crossword package for emacspeak -- see [1]https://github.com/tvraman/emacspeak

Good practical reason, then. Please get back to me with details of your testing.

-- hkp://keys.gnupg.net CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0

[tvraman]

Boruch Baum notifications@github.com writes: I suspect you need the (when (called-interactively-p 'interactive) check.

In my environment, I protected it with a similar (ems-interactive-p) check which is a function implemented by emacspeak.

Another alternative might be to entirely avoid advice, define crossword-insert as a new interactive command that does what you want, and bind it to [a-zA-Z] in crossword-mode that would entirely avoid all of this.

On 2021-01-21 10:53, T. V. Raman wrote:

Description: In line with your docstring that points out that the current advice is a bit of a kludge, here is one way to more clearly reflect the underlying intent of the advice --- see code below: it uses an around advice using the features of the original advice.el package.

How have you tested this and got it to work? I ask because my original implementation was an around advice and it would reliably immediately crash my emacs (circa versions 26/25) with some kind of recursion error IIRC. As in your code, my original never called ad-do-it.

It also avoids the need to call keyboard-quit at the end of the advice. This latter is particularly annoying at present as I try to speech-enable the crossword package for emacspeak -- see [1]https://github.com/tvraman/emacspeak

Good practical reason, then. Please get back to me with details of your testing.

-- hkp://keys.gnupg.net CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

--

Thanks,

--Raman ♈ Id: kg:/m/0285kf1 🦮

[Boruch-Baum]

On 2021-01-21 11:17, T. V. Raman wrote:

Boruch Baum notifications@github.com writes:

I suspect you need the (when (called-interactively-p 'interactive) check. In my environment, I protected it with a similar (ems-interactive-p) check which is a function implemented by emacspeak.

OK. Needs testing, but sounds simple, so I'll try it.

Another alternative might be to entirely avoid advice, define crossword-insert as a new interactive command that does what you want, and bind it to [a-zA-Z] in crossword-mode that would entirely avoid all of this.

Discussed in PR#8. TLDR; not i18n - friendly. I see the author of PR#8 just now in the past few minutes closed the PR, "saluting" me with the lines:

• "Okay whatever"

• "Have a nice life"

  AND

• "I'll just fork and do things how I like"

So, it looks like I scored a trifecta!

-- hkp://keys.gnupg.net CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0

[Boruch-Baum]

On 2021-01-21 11:17, T. V. Raman wrote:

I suspect you need the (when (called-interactively-p 'interactive) check.

Bummer. I was so hoping it would work, but it doesn't help. Tested and failed both on emacs26 and emacs28-snaphot-2020-09.

If you have any other ideas, that would be great. Otherwise, please close the PR.

I suppose also, at least for the short term, you could consider joining the author of PR#8 in his fork (but more politely, please). Keep in mind though, that while it will help with EMS, it will need extra work for i18n.

-- hkp://keys.gnupg.net CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0

[tvraman]

dont quite understand the I18N issue with respect to having a crossword-self-insert command, if it's hard-wiring a-zA-Z, you could always be more ambitious and bind it to all keys that invoke self-insert-command in crossword-mode.

From my experience advice for most packages is a great tool for experimentation, but perhaps not always the right thing for long-term use --- for the record, when I first released Emacspeak in 1995, RMS called me on the phone and spent nearly 30+ mins telling me about all the drawbacks of defadvice; I did take all his points into account, but still ended up using defadvice for emacspeak --- because for that use-case it was and continues to be the right solution. Note however that all of RMS' advice then helped me avoid many of the pitfalls inherent in using advice. --

Thanks,

--Raman ♈ Id: kg:/m/0285kf1 🦮

[tvraman]

See my just sent message re I18N. I wont fork -- if needed I'll do a custom solution inside emacspeak, since the current solution with keyboard-quit is basically unworkable / unusable for emacspeak.

If the I18N guy has walked away, I dont see why we cant start with the simple custom insert function for a-z:-)

--

Thanks,

--Raman ♈ Id: kg:/m/0285kf1 🦮

[Boruch-Baum]

On 2021-01-21 12:11, T. V. Raman wrote:

See my just sent message re I18N. I wont fork -- if needed I'll do a custom solution inside emacspeak, since the current solution with keyboard-quit is basically unworkable / unusable for emacspeak. If the I18N guy has walked away, I dont see why we cant start with the simple custom insert function for a-z:-)

See the thread there to get up-to-speed.

-- hkp://keys.gnupg.net CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0

[Boruch-Baum]

How did you manage to get the around advice working in emacs-speak, then? That solution might be applicable here. Have you seen the crash caused by your prior suggestions? How do you avoid that in emacs-speak?

-- hkp://keys.gnupg.net CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0

[tvraman]

Boruch Baum notifications@github.com writes:

There is a scary comment in advice.el that warns of potential problems around advicing core builtins.

That didn't bite emacspeak until around 2015 when there was a giant refactor in emacs, and as a consequence the interactive check provided by emacs hit an infinite loop in the semi.el package.

After studying some of the warnings in advice.el and fiddling around a bit, I worked out this implementation (I wouldn't recommend it for crossword since you only need one advice at present, and this solution is overkill).

But here it is:

https://github.com/tvraman/emacspeak/blob/master/lisp/emacspeak-preamble.el#L171

It's explained in this blog article: https://emacspeak.blogspot.com/2017/03/emacs-check-interactive-call-for.html I wrote it up a couple of years after I implemented it mostly so I'd remember what I had done.

How did you manage to get the around advice working in emacs-speak, then? That solution might be applicable here. Have you seen the crash caused by your prior suggestions? How do you avoid that in emacs-speak?

-- hkp://keys.gnupg.net CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

--

Thanks,

--Raman ♈ Id: kg:/m/0285kf1 🦮

[Boruch-Baum]

FYI: In beginning to work on this now, I see that the code in the blog post doesn't match the codebase. The blogpost version of the code performs an additional basis for setting variable 'result'. Maybe update the blog post for the new code?

On 2021-01-21 13:08, T. V. Raman wrote:

But here it is: https://github.com/tvraman/emacspeak/blob/master/lisp/emacspeak-preambl e.el#L171 It's explained in this blog article: https://emacspeak.blogspot.com/2017/03/emacs-check-interactive-call-for .html

-- hkp://keys.gnupg.net CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0

[ieure]

See my just sent message re I18N. I wont fork -- if needed I'll do a custom solution inside emacspeak, since the current solution with keyboard-quit is basically unworkable / unusable for emacspeak. If the I18N guy has walked away, I dont see why we cant start with the simple custom insert function for a-z:-)

You're welcome to try my fork. I'm not very familiar with emacspeak, but if there's something that can be done to make it work better, I'd be interested in making that happen -- but please file issues over there.

[Boruch-Baum]

This job looks done. Thanks for the guidance, TVR. If there are any hiccups, let me know.