skk-dev / ddskk

Daredevil SKK (Simple Kana to Kanji conversion program)
GNU General Public License v3.0
197 stars 41 forks source link

Cursor colour no longer changes to match hiragana/katakana mode. #190

Closed JordanAnthonyKing closed 3 years ago

JordanAnthonyKing commented 3 years ago

Hi, after a recent bump to this mode by doom-emacs the cursor colour no longer changes to match the input mode. I don't believe that this is custom behaviour implemented by Doom.

Can you confirm if this was deliberate?

conao3 commented 3 years ago

Maybe your issue comes from #189. Please test clean emacs (emacs -Q) and show minimum init.el, reproduce steps.

JordanAnthonyKing commented 3 years ago

Here is a screenshot of a fresh emacs install:

image

I have:

Toggling hiragana and kana via q does not change the colour of the cursor. Candidate text does match the colour shown in the modeline however.

Back to my doom config: I have configured straight to pull the master branch of this repo and the same is true:

image

Thanks

JordanAnthonyKing commented 3 years ago

An extra note, when opening emacs after updating ddskk this message is shown in the minibuffer:

Error in post-command-hook (ccc-update-buffer-local-frame-params): (void-function facement-color-equal)

So yes, it would seem that this was introduced by #189

conao3 commented 3 years ago

Confirmed. cursor color now does not change. your message, Error in post-command-hook (ccc-update-buffer-local-frame-params): (void-function facement-color-equal) is directly indicate this issue. (I cannot get this message) But there're no facemenu-color-equal I think...? (Stopped using facemenu functions and removed its dependencies in #189.)

If we get backtrace, this debugging could go forward. If you eval (setq debug-on-error t) before that message, your Emacs show some backtrace?

JordanAnthonyKing commented 3 years ago

Although I can't seem to get any backtrace when emacs loads, if I call ccc-set-buffer-local-cursor-color manually I get this:

Debugger entered--Lisp error: (void-function facemenu-read-color)
  facemenu-read-color("Cursor color: ")
  byte-code("\301\30\302\10!)C\207" [prompt "Cursor color: " facemenu-read-color] 2)
  call-interactively(ccc-set-buffer-local-cursor-color record nil)
  command-execute(ccc-set-buffer-local-cursor-color record)
  counsel-M-x-action("ccc-set-buffer-local-cursor-color")
  #f(compiled-function (x) #<bytecode -0x16d723f3d35b6b80>)("ccc-set-buffer-local-cursor-color")
  ivy-call()
conao3 commented 3 years ago

I confirmed ddskk cursor no longer changes its color but my ccc-set-buffer-local-cursor-color works well. I tested like below step. Please try it.

  1. Save below file as ~/.debug.emacs.d/ddskk/init.el
    
    ;; ~/.debug.emacs.d/ddskk/init.el

;; you can run like 'emacs -q -l ~/.debug.emacs.d/ddskk/init.el' (when load-file-name (setq user-emacs-directory (expand-file-name (file-name-directory load-file-name))))

(prog1 "leaf" (custom-set-variables '(package-archives '(("org" . "https://orgmode.org/elpa/") ("melpa" . "https://melpa.org/packages/") ("gnu" . "https://elpa.gnu.org/packages/")))) (package-initialize) (unless (package-installed-p 'leaf) (package-refresh-contents) (package-install 'leaf)))

(leaf *initialize-emacs :custom ((debug-on-error . t) (init-file-debug . t)))

(leaf skk :ensure ddskk :bind (("C-\" . skk-mode)) :custom ((default-input-method . "japanese-skk")))


2. Run Emacs with loading that file
    `emacs -q -l ~/.debug.emacs.d/ddskk/init.el`
3. After emacs launched, `M-x ccc-set-buffer-local-cursor-color red`
4. I confirmed cursor color change into red without any errors.
JordanAnthonyKing commented 3 years ago

I'm unsure how you're running ccc-set-buffer-local-cursor-color from M-x with a parameter. Is there a prompt after doing M-x ccc-set-buffer-local-cursor-color RET? Running M-x eval-expression and then (ccc-set-buffer-local-cursor-color red) also doesn't work.

If these functions require a parameter they shouldn't really be invokable from M-x anyway right?

conao3 commented 3 years ago

Yes, actual key sequence, M-x ccc-set-buffer-local-cursor-color RET red. And it makes my cursor red. Please make sure remove ~/.debug.emacs.d/ddskk/elpa directory if you have already.

JordanAnthonyKing commented 3 years ago

Apologies, I was testing on doom emacs with ivy. On vanilla emacs M-x ccc-set-buffer-local-cursor-color RET red does indeed work as expected.

conao3 commented 3 years ago

OK, we can go forward next step. Why ccc-set-buffer-local-cursor-color works well, ddskk cursor no longer changes its color? I'll look into this but I don't have much time on this issue...

For now, I'll revert #189, please test the reverted version ddskk. (maybe after 2 or 3 hours, MELPA will give you that version)

JordanAnthonyKing commented 3 years ago

Good to hear 🙂 I'm about to perform a fresh install of doom. I will let you know.

JordanAnthonyKing commented 3 years ago

Has #189 been reverted? I still see it in the commit history?

conao3 commented 3 years ago

I miss clicked Close pull-request! Reopen and merged! :) Please test after 2 or 3 hours later.

JordanAnthonyKing commented 3 years ago

Hi, I don't believe reverting #189 has fixed the issue. Looking at doom it seems that the issue was introduced some time before commit cec99365fb3ad725c44514b918696ab153596b8d

tats commented 3 years ago

A current workaround is evaluating (require 'facemenu), because recent Emacs 28 no longer preload facemenu.el.

The fix by https://github.com/skk-dev/ddskk/pull/189 looks correct, so I believe reverting is unneeded.

Please note that ccc.el is provided by the ccc package instead of the ddskk package, so take care of the version of the ccc package.

JordanAnthonyKing commented 3 years ago

I agree reverting was probably unnecessary, however I was running emacs 28 with the latest version of this entire repo, ddskk, ccc etc. and was not seeing the desired behaviour.

Shall I try again after the revert has been reverted?

tats commented 3 years ago

The current code needs (require 'facemenu) for Emacs 28 because of reverting https://github.com/skk-dev/ddskk/pull/189.

Also note that the inline functions are expanded into the caller, so you need to recompile ddskk with the updated ccc if the inline functions in ccc.el are updated.

JordanAnthonyKing commented 3 years ago

I can confirm this works now.