Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] need update for latest jsonrpc accessor change: jsonrpc--request-continuations #687

Closed
3 tasks done
garyo opened this issue Dec 30, 2023 · 9 comments
Closed
3 tasks done
Labels
bug Something isn't working

Comments

@garyo
Copy link

garyo commented Dec 30, 2023

Thank you for the bug report

  • I am using the latest version of doom-modeline related packages.
  • I checked FAQ.
  • You may also try reproduce the issue using clean environment and minimal configurations with
    the command emacs -Q.

Bug description

In emacs commit dceffddbfe7, jsonrpc--request-continuations is replaced with jsonrpc--continuations in lisp/jsonrpc.el. As of doom-modeline a312ea8, doom-modeline-segments.el uses the old accessor function.

This trivial patch fixes it for me:

diff --git a/doom-modeline-segments.el b/doom-modeline-segments.el
index 1182d58..ad93827 100644
--- a/doom-modeline-segments.el
+++ b/doom-modeline-segments.el
@@ -193,7 +193,7 @@
 (declare-function iedit-find-current-occurrence-overlay "ext:iedit-lib")
 (declare-function iedit-prev-occurrence "ext:iedit-lib")
 (declare-function image-get-display-property "image-mode")
-(declare-function jsonrpc--request-continuations "ext:jsonrpc" t t)
+(declare-function jsonrpc--continuations "ext:jsonrpc" t t)
 (declare-function jsonrpc-last-error "ext:jsonrpc" t t)
 (declare-function lsp--workspace-print "ext:lsp-mode")
 (declare-function lsp-describe-session "ext:lsp-mode")
@@ -2073,7 +2073,7 @@ mouse-1: Reload to start server")
   "Get count of pending eglot requests to SERVER."
   (if (fboundp 'jsonrpc-continuation-count)
       (jsonrpc-continuation-count server)
-    (hash-table-count (jsonrpc--request-continuations server))))
+    (hash-table-count (jsonrpc--continuations server))))

 (defvar-local doom-modeline--eglot nil)
 (defun doom-modeline-update-eglot ()

but I expect the actual fix will need to cover both cases for compatibility.

Steps to reproduce

Try to use eglot with doom-modeline, using latest master emacs.

Expected behavior

Should work without error.

OS

Windows

Emacs Version

30

Emacs Configurations

No response

Error callstack

No response

Anything else

No response

@garyo garyo added the bug Something isn't working label Dec 30, 2023
@seagle0128
Copy link
Owner

Thank you for the kind reminder!

@uqix
Copy link

uqix commented Jan 1, 2024

This fix results in error on Emacs 29.1:

error in process filter: doom-modeline--eglot-pending-count: Symbol’s function definition is void: jsonrpc--continuations
error in process filter: Symbol’s function definition is void: jsonrpc--continuations

@seagle0128
Copy link
Owner

seagle0128 commented Jan 2, 2024

@uqix I reverted 9773ef7. Sorry for the inconvenience!

@garyo In 9773ef7, the compatibility has been considered.

@pcompassion
Copy link

Hi, should I unpin the package to use the fixed version?

@seagle0128
Copy link
Owner

@pcompassion pls use the latest version.

@pcompassion
Copy link

@seagle0128 thanks for the response, but i tried doing

(unpin! doom-modeline) in my packages.el
and even changing the

(unless (modulep! +light)
(package! doom-modeline :pin "93f240f7a0bf35511cfc0a8dd75786744b4bcf77"))
to

(unless (modulep! +light)
(package! doom-modeline))

and doing doom-sync, doens't give me the latest version..

@seagle0128
Copy link
Owner

@pcompassion I think you should file an issue for doom emacs.

@cole-jacobs
Copy link

@pcompassion maybe you need doom sync -u [1]?

[1] https://github.com/doomemacs/doomemacs/blob/master/docs/getting_started.org#the-bindoom-utility

You’ll need doom sync -u if you override the recipe of package installed by another module.

@pcompassion
Copy link

@seagle0128 @pcompassion thanks for the responses, i'll handle it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants