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

Fix: update eaf layer #14681

Merged
merged 1 commit into from
Apr 19, 2021
Merged

Conversation

dalanicolai
Copy link
Contributor

The layer got broken due to some minor changes in the eaf package by the eaf
developers. This PR fixes the layer to a working state.

The layer got broken due to some minor changes in the eaf package by the eaf
developers. This commit fixes the layer to a working state.
@dalanicolai
Copy link
Contributor Author

Or actually I think it got broken by the recent big refactor operation...

@@ -236,20 +235,19 @@
(lambda (prompt)
(if (derived-mode-p 'eaf-mode)
(pcase eaf--buffer-app-name
((or
(and "browser"
(guard (string= (eaf-call-sync "call_function" eaf--buffer-id "is_focus") "True")))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(guard (string= (eaf-call-sync "call_function" eaf--buffer-id "is_focus") "True")))

to

(guard (not (string= (eaf-call-sync "call_function" eaf--buffer-id "is_focus") "True"))))

Should fix this.

Copy link
Contributor Author

@dalanicolai dalanicolai Apr 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you mean I should just add the not in the original code (before the PR)? Anyway, I have tried that but that also does not seem to work (ESC does exit insert mode).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry... I think it does work, there is another reason why it seems broken. I don't know exactly how eaf reparenting works, but somehow (on Ubuntu) the browser window seems not to get well embedded in Emacs, but instead it opens in its own separate window. But from its position it looks to be embedded in Emacs, however when I select the browser window, that window gets active instead of the Emacs window (so that the keybindings don't seem to work). On Fedora it all seems to work fine (I am switching a little between Fedora and Ubuntu, because mit-scheme does not build in Fedora. Anyway.. this makes things a little more confusing.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will check a little more and then update the PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Congrats with your new role in the Spacemacs team!

Copy link
Collaborator

@lebensterben lebensterben Apr 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Key binding issue aside, I've trouble with SPC m d for toggling dark-mode.
It works like 10% of time and for 90% of time it complains "cannot call function: is_focus".

P.S. EAF never fully worked on my computer so I cannot fully test it.

Copy link
Collaborator

@smile13241324 smile13241324 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@smile13241324 smile13241324 merged commit 623c75d into syl20bnr:develop Apr 19, 2021
(_ (kbd "SPC")))
(kbd "SPC"))))

;; The following lines create the major-mode leader key emulation map
;; in a similar way as how it was done in the evil-integration example
(setq eaf-evil-leader-for-major-keymap (make-sparse-keymap))
(define-key eaf-evil-leader-for-major-keymap (kbd "h") 'eaf-open-browser-with-history)
(define-key eaf-evil-leader-for-major-keymap (kbd "d") 'eaf-toggle-dark-mode)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is defined in funcs.el and now it's not used.
Do we need to remove it? But it works 100% of time on my computer....

Copy link
Contributor Author

@dalanicolai dalanicolai Apr 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure that function works? Because here it did not. I did not remove it, but replace it with the working version of that function (see below)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can only view PDF file in EAF on my computer. And the only working function is to toggle dark mode. After this commit that stopped working.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the it should not be this line.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I meant is

(defun eaf-toggle-dark-mode ()

That is not used after this PR. And that function works for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha okay, I forgot that I had added that function myself. But I am using mainly the eaf-browser, and there this function stopped working while that new function does work. So I guess maybe we require both functions. I will look into it a little more then.

dalanicolai added a commit to dalanicolai/spacemacs that referenced this pull request Apr 20, 2021
This PR reimplements the refactor changes by @lebensterben but with his fix as
he suggested [here](syl20bnr#14681 (comment))
dalanicolai added a commit to dalanicolai/spacemacs that referenced this pull request Apr 20, 2021
This PR reimplements the refactor changes by @lebensterben but with his fix as
he suggested [here](syl20bnr#14681 (comment))
dalanicolai added a commit to dalanicolai/spacemacs that referenced this pull request Apr 20, 2021
This PR reimplements the refactor changes by @lebensterben but with his fix as
he suggested [here](syl20bnr#14681 (comment))
dalanicolai added a commit to dalanicolai/spacemacs that referenced this pull request Apr 20, 2021
This PR reimplements the refactor changes by @lebensterben but with his fix as
he suggested [here](syl20bnr#14681 (comment))
dalanicolai added a commit to dalanicolai/spacemacs that referenced this pull request Apr 20, 2021
This PR reimplements the refactor changes by @lebensterben but with his fix as
he suggested [here](syl20bnr#14681 (comment))
dalanicolai added a commit to dalanicolai/spacemacs that referenced this pull request May 8, 2021
This PR reimplements the refactor changes by @lebensterben but with his fix as
he suggested [here](syl20bnr#14681 (comment))
smile13241324 pushed a commit that referenced this pull request May 8, 2021
This PR reimplements the refactor changes by @lebensterben but with his fix as
he suggested [here](#14681 (comment))
wang-d pushed a commit to wang-d/spacemacs that referenced this pull request Jul 22, 2021
This PR reimplements the refactor changes by @lebensterben but with his fix as
he suggested [here](syl20bnr#14681 (comment))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants