ecb-home / ecb

ECB Main Repository
Other
54 stars 11 forks source link

emacs 29.x #43

Open tec-leo opened 8 months ago

tec-leo commented 8 months ago

I have no deeper knowledge about ecb code, so please check carefully before merging. Some comments about the changes are in the new file changes.txt. If you find something's wrong or bad or there are better solutions please keep me informed.

belden commented 6 months ago

I see this is related to https://github.com/ecb-home/ecb/issues/42 - I wonder if there are any plans to merge this?

I tried installing ecb via melpa today. I'm using Emacs 29.3, and ran into significant errors when trying to install the package. Hopefully this PR fixes it, but I'm unable to run make locally on tec-leo:t42-emacs29 to build and test for myself.

tec-leo commented 6 months ago

Pull request is out but I don't know if and when it will be handled.

You ran make on Linux or Windows? In what shell?

belden commented 5 months ago

@tec-leo Oh, sorry not to have included that originally. I'm using bash 5.2.26 on OSX.

tec-leo commented 5 months ago

Please make an issue first. As a workaround you can compile all ecb files directly in Emacs. go to the ecb directory and execute: C-u 0 M-x byte-recompile-directory this will compile all the .el files in the directory and in all subdirectories below (I think ecb2 is not used anymore, maybe a test?). C-u 0 in the beginning just avoids Emacs asking about every .el file that does not have a .elc file.

xtian-kc commented 1 month ago

Trying tec-leo:t42-emacs29 with Emacs 29.4 on Debian, I'm getting the attached trace on startup. The unknown symbol "ecb-tree-buffer-face" used in "tree-buffer.el" is defined in "ecb-face.el", but it does not seem to be picked up (or not early enough). Is there anything special which I have missed to configure (ecb directory is in load path), for the "ecb-face.el" to be evaluated on startup? I'd be happy to test any ideas, just let me know. ecb-backtrace.txt

tec-leo commented 1 month ago

I am also not sure what's the reason. Have you byte compiled the ecb-directory before? If not you can do it with ecb-byte-compile after ecb activated.

But it you want to help testing, could you please insert the following alias in treebuffer.el (just at the beginning of the file, easy to find):

;; GNU Emacs (defalias 'tree-buffer-face 'ecb-tree-buffer-face) (defalias 'tree-buffer-facep 'facep) (defalias 'tree-buffer-line-beginning-pos 'line-beginning-position) (defalias 'tree-buffer-line-end-pos 'line-end-position) (defalias 'tree-buffer-frame-char-width 'frame-char-width) (defalias 'tree-buffer-window-display-height 'window-text-height)

and test again, but do not byte compile before or the effect will not be visible. Please let me know if this solved the problem.

thanks for helping

xtian-kc commented 1 month ago

Thank you very much for looking into this. I haven't byte-compiled ECB yet, since due to the error, I cannot activate ECB.

I've added the defalias at the place you indicated, but it did not change the outcome, same error.

I have quite a lengthy .emacs file, so I guess I will kick everything but CEDET and ECB and try again to minimize possible interference from other packages.

Do you have an idea which kind of settings / packages / ... could possibly mess with the visibility of the "ecb-tree-buffer-face" symbol?

FWIW, I'm on https://github.com/tec-leo/ecb.git commit 1684ba51980b4f2523a78804fb4b352b49cb9844 "removed semantic version", fresh checkout w/ just the change you mentioned applied to tree-buffer.el

Best

Edit: Stripped down .emacs to only packages, CEDET and ECB, but still no luck. I've messed around with some .el files and have a basic understanding of Lisp, but I am by no means a Lisp coder or well-versed debugging Emacs. The availability of ECB though is one of the main reasons for me to stick with Emacs, so I'll be thankful for any debugging hints. Would be great to get ECB flying again! :)

Edit2: This is Emacs 29.4 from the Debian-Backports for bookworm, but I don't think that really matters

Edit3: Double-checked that there are no *.elc files in the ECB directory

tec-leo commented 1 month ago

Don't worry, we will get it to work.

Just as a test, could you try to comment out the two usages of ecb-tree-buffer-face in treebuffer.el?

(defun tree-buffer-merge-face (face start end &optional text) "Merge FACE either to a buffer-part or to TEXT. In both cases START and END define the region which should be faced. The FACE is merged, i.e. the values of all face-attributes of FACE take effect and the values of all face-attributes of the buffer-part or TEXT which are not set by FACE are preserved.

If always returns TEXT (if not nil then modified with FACE)." (if (null face) text (alter-text-property start end 'face (lambda (current-face) (let ((cf (cl-typecase current-face ; (ecb-tree-buffer-face (list current-face)) (list current-face) (otherwise nil))) (nf (cl-typecase face ; (ecb-tree-buffer-face (list face)) (list face) (otherwise nil)))) ;; we must add the new-face in front of ;; current-face to get the right merge! (if (member face cf) cf (append nf cf)))) text) text))

There is still a lot of old and unused code in ecb, and I also have not to much knowledge or experience of ecb because I am not a developer. I just try to fix as much as I can, there a lot of warnings I will try to remove the next time. Next week I cannot work on it, but later.

xtian-kc commented 1 month ago

Did the change you requested, now we're failing with a different trace. That trace, however, might be expected at that point, since we're stealing arguments? Or maybe unrelated? Let's see. :) ecb-backtrace-with-commented-ecb-tree-buffer-face.txt

tec-leo commented 1 month ago

I can check this next week. It would be helpful if you could prepare a minimalistic .emacs ini file which shows the error in the trace so that I can reproduce it on my machine.

xtian-kc commented 1 month ago

The attached .emacs file is enough to reproduce the issue on my machine. dot-emacs.stripped.txt

If this does not reproduce the error over there at your end, then I guess we might need to figure out what the difference in our setups is.

tec-leo commented 1 month ago

I could not reproduce it on Windows 11, but on linux (open suse tumbleweed) I got errors, but different ones. After moving the .emacs.d directory to a different location it worked, maybe it's best to install the packages from scratch. I got just complains about different cedet versions, to I had to remove all .elc file and byte-compile again. btw. this is not necessary any more: ;(package-initialize) ;exec-path-from-shell-initialize)

Let me know if it works now. There are still warning and undefined variables, I will try to fix as much as possible.

xtian-kc commented 1 month ago

Hi again, thanks for returning to this. I've moved my old .emacs.d/ out of the way and reinstalled my packages from scratch (will have been a good thing to do anyway). I'm now getting a different error, during GNU global version check (I have it installed). I've further stripped down my .emacs file, which I am attaching along with the debug-init trace I'm now getting. A web search shows quite some instances of similar issues in other projects, so there may be a common pitfall somewhere. dot-emacs-stripped.txt emacs-backtrace_cedet-gnu-global-version-check.txt

Edit: I've temporarily removed my "global" debian package (which provides /usr/bin/global), and the error is gone. I can then 'ecb-activate. Alas, no /usr/bin/global then. =) Seems to me that there is an issue with the 'global' version check, which if the executable is not installed does not show. HTH

tec-leo commented 1 month ago

maybe be a problem of Debinan? I have installed global on open suse and the cedet-gnu-global-version-check returned the same as global --version: GNU Global 6.6.13 - Good enough for CEDET.

Anyway, I will continue the remove warnings and undefined symbols.

xtian-kc commented 1 month ago

Do you mind pasting the output of the global --version that you get? On Debian/Stable it is

global (Global) 6.6.9
Powered by Berkeley DB 1.85 and SQLite3 3.40.1.
Copyright (c) 1996-2022 Tama Communications Corporation
License GPLv3+: GNU GPL version 3 or later <http://www.gnu.org/licenses/gpl.html>
This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

I will try and call cedet-gnu-global-version-check manually on my global binary, maybe I can find something here. It would be nice to have the output from the SuSE variant, though.

Thanks for working on ECB, means a lot! :)

tec-leo commented 1 month ago

open suse konsole (terminal): global --version lobal (GNU Global) 6.6.13 Powered by Berkeley DB 1.85 and SQLite3 3.8.7.1. Copyright (c) 1996-2024 Tama Communications Corporation License GPLv3+: GNU GPL version 3 or later http://www.gnu.org/licenses/gpl.html This is free software; you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law.

cedet-gnu-global-version-check GNU Global 6.6.13 - Good enough for CEDET.

But the version number is not the problem, cedet just calls the same command global --version if global is not defined it returns nil and this cannot be converted to a string with version-to-list(nil)

xtian-kc commented 1 month ago

My thinking was along the lines of a parse issue when getting the version number from the 'global --version' output due to differing outputs from global.

If I uninstall 'global', this gets detected properly and ECB works (w/o global, of course). If I have it installed, I get the above error message during startup. I'll try and dig into this, maybe it'll turn something up.

xtian-kc commented 1 month ago

Checking cedet-gnu-global-version-check I found that it actually checks for (GNU Global) substring in the output, while the Debian version of globalonly outputs (Global). This one is for either Debian or the CEDET guys then.

xtian-kc commented 3 weeks ago

I've been using this branch for a while now and just wanted to report back. Looking good for me, I've not noticed any failures yet. Thanks a 1e6!

tec-leo commented 2 weeks ago

Sounds good, thank you for the feedback.