search

lokedhs / gnu-apl-mode

GNU APL mode for Emacs
GNU General Public License v3.0
94 stars 18 forks source link

Allow prefix key for APL symbol input to be customised #37

Closed phikal closed 4 years ago

phikal commented 4 years ago

I prefer to use the ` key for symbol input, as I'm used to it from AucTeX, but the code seems to have hard-coded . as the prefix key.

As I see no reason why this should be the case, I hope this change can help people like me, who would prefer a different key.

lokedhs commented 4 years ago

Thank you for the pull request. Unfortunately, for the fix to work the variable gnu-apl-key-prefix needs to be set before the file gnu-apl-interactive.el is loaded.

To fix this, the defcustom for gnu-apl-prefix needs to have a :set function that updates the quail definition so that the value can be changed. You can take a look at the implementation of gnu-apl-mode-map-prefix which had a similar problem.

If this gets implemented, I'll be happy to merge the code.

phikal commented 4 years ago

Will do, I just have one question: What's the rationale behind using macrolet to define make-quail-define-rules? Is it something "important" or would you be ok with changing it?

phikal commented 4 years ago

~Hope this works. Didn't get to test it extensively today, but in case it doesn't, I'll get around to fixing it tomorrow.~

Ok, it doesn't work, it doesn't work, will fix.

phikal commented 4 years ago

Seems to work now.

lokedhs commented 4 years ago

Thank you for the update, and sorry for taking so long before testing your code.

I found some issues with the code. First and foremost, if the variable gnu-apl-key-prefix is not set, the Quail mode will not be generated at all. If it's not set, the default behaviour of using a period should still be used.

Secondly, the use of eval isn't very pretty. I will commit an update to this file that clean up this code a bit, removing the use of the macro. That should make your code easier to integrate.

lokedhs commented 4 years ago

OK, I looked at the code and I now know why I used a macro. It's because quail-define-rules is a macro itself, so in order to be able to regenerate the rules, the eval is going to be needed. It's ugly, but there seems to be no other public API to access the quail config.

Thus, if the behaviour when the config is not set is fixed, I can merge it.

phikal commented 4 years ago

Are you sure, I'm currently testing quail-defrule and it seems to do the same thing. Either way, I'll see what works.

phikal commented 4 years ago

I seem to have been mistaken, at least partially: quail-defrule works, but I can't find a way to disable the old keybindings, at least in quail.el. Do you know of something?

phikal commented 4 years ago

I tried using quail-map-from-table, and it seems to work. I can change the prefix key mid-session, and the old key is forgotten. The code is ok, but not too pretty.

phikal commented 4 years ago

bump?

lokedhs commented 4 years ago

Sorry for not replying earlier. I did indeed forget about it.

It looks good now, and I have just one more request: Can you remove the tab characters and replace with regular indentation using spaces? Other than that I can merge.

phikal commented 4 years ago

Oops, didn't notice that. It might be worth adding a .dir-locals.el, to avoid that in the future.

lokedhs commented 4 years ago

Thanks. It looks good now.