Reimplement eaf refactor with fix #14684
Conversation
This PR reimplements the refactor changes by @lebensterben but with his fix as he suggested [here](syl20bnr#14681 (comment))
| @@ -131,6 +131,7 @@ | |||
| ("-" . "insert_or_zoom_out") | |||
| ("=" . "insert_or_zoom_in") | |||
| ("0" . "insert_or_zoom_reset") | |||
| ;; ("d" . "insert_or_dark_mode") | |||
There was a problem hiding this comment.
I thought I do keep it here (but commented out). It can be made working again with some modifications to the eaf package (the same modifications I made there before)
|
Ah, I see this PR consists of two commits now. I had created a PR but forgot to sync with develop first. So then I created a new PR (in a new branch) and just merged the fixes from this previous PR/branch. Is it easy for you to squash it/cherry pick? I have to look up again how to do that (this time when trying to rebase interactively, it asks me if I want to continue with a merge within the range). If I had had more time I would go sort it out, but I think for you this is easy (otherwise let me know of course). |
dalanicolai
force-pushed
the
reimplement-eaf-refactor-with-fix
branch
2 times, most recently
from
5f0192c
to
72aef58
Compare
This PR reimplements the refactor changes by @lebensterben but with his fix as he suggested [here](syl20bnr#14681 (comment))
dalanicolai
force-pushed
the
reimplement-eaf-refactor-with-fix
branch
from
72aef58
to
354ed12
Compare
|
the real issue is not how you integrate it into spacemacs. EAF is still in very early stage. Just look at code like this. It just feels weird. It needs some major refactors. I spent much time trying to make the code short, fast, and idiomatic. This eaf.el had >2000 lines. |
|
Or something like this He's using string like enum. But comparing string is much less efficient than comparing symbols or integers. |
|
Yes I saw your bug-reports now at EAF, I have created two more myself too. But, despite the quality of the code, the browser and pdf-reader work (almost) flawlessly on my Fedora, and they are both really nice apps. Very good/helpful that you can advise them a little, but they did very nice work already of course. He already voted up your suggestions... |
Haha, I am still only a hobby programmer. It is hard work for me to evaluate/judge this code... but of course I am trying to understand and learn from all feedback (as far as time allows) |
|
Hmmm looks like you have made a rebase on the branch, I cannot properly squash or merge it. |
|
@dalanicolai is it ok for you to reproduce your changes on a new branch? If not I could also just copy them over and sign you in the commit message, if this is ok for you. Just drop me a line. |
|
@smile13241324 I have created the new PR (#14747) as requested, so this one can be closed. |
|
New one is merged. |
Thanks for merging the eaf update quickly @smile, but @lebensterben and I were still planning to make these little changes (see commit message).