Adjust eaf layer to newer eaf package #14992

lesliebinbin · 2021-08-15T10:12:14Z

Fix leader key for eaf camera, it should be eaf-open-camera instead of eaf-camera
Refer to a recent commit: emacs-eaf/emacs-application-framework@044b0b8, we should now require each application seperately

dalanicolai · 2021-08-16T11:22:52Z

Thanks for creating this PR. However, I think we should probably make the list of apps to load configurable somehow. So I suggest that you make the list a layer variable in config.el and then use that variable in place of the list in packages.el. Subsequently, this should also get updated in the documentation (README).

Would you like to take care of that? Then also, it is usually preferred/required, to submit a single commit (here is a guide that explains how to squash with magit, and here is a guide about updating the PR, before or after squashing).

Of course, this might be a little too much to ask if your time is limited. In that case, simply let us know, and we will take care of it.
Also, you can always ask for help/advice on Spacemacs gitter.

Anyway, thanks again!

layers/+tools/eaf/packages.el

lebensterben

LGTM

MatthewZMD · 2021-08-16T21:08:52Z

layers/+tools/eaf/packages.el

+                                          'eaf-terminal
+                                          'eaf-netease-cloud-music
+                                          'eaf-video-player
+                                          'eaf-js-video-player


The latest EAF has dropped the support to js-video-player, feel free to remove it.

The reason is explained here

MatthewZMD · 2021-08-16T21:16:43Z

On top of my comment, I noticed that:

the EAF repo url in spacemacs is still manateelazycat/emacs-application-framework.

The EAF repos have merged to their own Github org: emacs-eaf.

Although the old url will still work and redirect to the new one, I'd suggest, as a good practice, to set the remote to https://github.com/emacs-eaf/emacs-application-framework.git instead.

See more here

These are no longer EAF dependecies, as EAF will maintain its own fork of them from now on.

(defun eaf/init-ctable ()
  (use-package ctable))
(defun eaf/init-deferred ()
  (use-package deferred))
(defun eaf/init-epc ()
  (use-package epc))
;; (defun eaf/init-s ()
;;   (use-package s))

These lines can be safely deleted.

See the conversation here for details.

All eaf-setq needs to change to setq as per here
A number of keybindings declared maybe should be set only if the apps are present.

lebensterben

Please make changes according to #14992 (comment)

lesliebinbin · 2021-08-21T05:22:01Z

On top of my comment, I noticed that:

the EAF repo url in spacemacs is still manateelazycat/emacs-application-framework.

The EAF repos have merged to their own Github org: emacs-eaf.

Although the old url will still work and redirect to the new one, I'd suggest, as a good practice, to set the remote to https://github.com/emacs-eaf/emacs-application-framework.git instead.

See more here

These are no longer EAF dependecies, as EAF will maintain its own fork of them from now on.
(defun eaf/init-ctable ()
  (use-package ctable))
(defun eaf/init-deferred ()
  (use-package deferred))
(defun eaf/init-epc ()
  (use-package epc))
;; (defun eaf/init-s ()
;;   (use-package s))
These lines can be safely deleted.

See the conversation here for details.

All eaf-setq needs to change to setq as per here

A number of keybindings declared maybe should be set only if the apps are present.

But if we remove these, the macos user might get errors, since the spacemacs in macos still use the eaf release in May

lebensterben · 2021-08-21T05:01:31Z

layers/+tools/eaf/packages.el

+        'eaf
+        (dolist (app eaf-apps)
+          (require app nil 'noerror))
+      )


trailing ) should not take a new line.

lebensterben · 2021-08-21T05:02:52Z

layers/+tools/eaf/packages.el

-                                          )))
-                      (dolist (app eaf-apps)
-                        (require app nil 'noerror))))
+      (with-eval-after-load


there's no point of having with-eval-after-load 'foo in :init sectiion of (use-package foo ..) form.

move it to :config and remove with-eval-after-load

lebensterben · 2021-08-21T05:17:13Z

layers/+tools/eaf/config.el

+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+(defcustom eaf-apps "Eaf Applications"


Multiple issues:

For defcustom, the second positional arg is the default value and the third positional arg is the docstring.

The keyword arg :type has an invalid value (symbol). At very least, it should be 'symbol. Here you should use '(set (const eaf-jupyter) (const eaf-browser) ...). See https://www.gnu.org/software/emacs/manual/html_node/elisp/Simple-Types.html

If you intended to make this a custom variable, you should not use defcustom directly.
Instead you should use spacemacs|defc, which is a wrapper of defcustom.

A better docstring is required. And you also need to document it in layer's README.org.

lebensterben · 2021-08-21T05:19:31Z

layers/+tools/eaf/config.el

@@ -0,0 +1,46 @@
+;;; packages.el --- eaf configuration file


It's not packages.el

The copyright year of this file is 2021.

lebensterben · 2021-08-21T05:37:39Z

Using the snippet from https://github.com/emacs-eaf/emacs-application-framework#1-download-eaf, won't correctly install eaf:

(quelpa '(eaf :fetcher github
              :repo  "emacs-eaf/emacs-application-framework"
              :files ("*")))

It shows

Compiling file /home/lucius/.emacs.d/elpa/27.2/develop/eaf-20210821.113/eaf.el at Sat Aug 21 01:34:44 2021
Entering directory ‘/home/lucius/.emacs.d/elpa/27.2/develop/eaf-20210821.113/’
eaf.el:167:1:Error: Cannot open load file: No such file or directory, eaf-epc
Leaving directory ‘/home/lucius/.emacs.d/elpa/27.2/develop/eaf-20210821.113/’

lebensterben · 2021-08-21T06:35:46Z

layers/+tools/eaf/README.org

-in their terminal. If the script does not work for you then [[https://github.com/manateelazycat/emacs-application-framework#dependency-list][install the
-required dependencies manually]].
+Currently the command =SPC SPC eaf-install-dependencies= no longer works, please
+refer to [[https://github.com/manateelazycat/emacs-application-framework#dependency-list][install the required dependencies manually]].


please update all the links since the repository has been moved

lebensterben · 2021-08-21T06:37:23Z

I won't merge this PR until it's verified to be installable.

Currently it's broken.

MatthewZMD · 2021-08-21T07:09:24Z

But if we remove these, the macos user might get errors, since the spacemacs in macos still use the eaf release in May

Is it possible to update the release?

On the other hand, we're actually planning to do real releases in the near future, after things are kinda more settled.

MatthewZMD · 2021-08-21T07:16:42Z

Using the snippet from https://github.com/emacs-eaf/emacs-application-framework#1-download-eaf, won't correctly install eaf:

(quelpa '(eaf :fetcher github
              :repo  "emacs-eaf/emacs-application-framework"
              :files ("*")))

It shows

Compiling file /home/lucius/.emacs.d/elpa/27.2/develop/eaf-20210821.113/eaf.el at Sat Aug 21 01:34:44 2021
Entering directory ‘/home/lucius/.emacs.d/elpa/27.2/develop/eaf-20210821.113/’
eaf.el:167:1:Error: Cannot open load file: No such file or directory, eaf-epc
Leaving directory ‘/home/lucius/.emacs.d/elpa/27.2/develop/eaf-20210821.113/’

Ouch, this quelpa recipe is from the pre-split period not so long ago, we haven't updated it, I've removed from the README until a working recipe is emerged. But I only use use-package, not so sure what's the best way to write up a recipe at the moment, it is still an ongoing discussion.

The eaf-epc.el is located in eaf/core directory, do you know why isn't it loaded in the load-path

MatthewZMD · 2021-08-21T07:21:55Z

layers/+tools/eaf/README.org

+- eaf-markdown-previewer
+- eaf-camera
+
+To customise applications, please set =eaf=app=


Is =eaf=app= a typo?

Yep, thx 😄

lesliebinbin · 2021-08-21T07:27:50Z

Using the snippet from https://github.com/emacs-eaf/emacs-application-framework#1-download-eaf, won't correctly install eaf:
(quelpa '(eaf :fetcher github
              :repo  "emacs-eaf/emacs-application-framework"
              :files ("*")))
It shows
Compiling file /home/lucius/.emacs.d/elpa/27.2/develop/eaf-20210821.113/eaf.el at Sat Aug 21 01:34:44 2021
Entering directory ‘/home/lucius/.emacs.d/elpa/27.2/develop/eaf-20210821.113/’
eaf.el:167:1:Error: Cannot open load file: No such file or directory, eaf-epc
Leaving directory ‘/home/lucius/.emacs.d/elpa/27.2/develop/eaf-20210821.113/’
Ouch, this quelpa recipe is from the pre-split period not so long ago, we haven't updated it, I've removed from the README until a working recipe is emerged. But I only use use-package, not so sure what's the best way to write up a recipe at the moment, it is still an ongoing discussion.

The eaf-epc.el is located in eaf/core directory, do you know why isn't it loaded in the load-path

Not quite sure but currently I set it as a custom layer and install-eaf.py works, is it because there is difference when we put

(defconst eaf-packages
  '(ctable
    deferred
    epc
    (eaf :location (recipe
                    :fetcher github
                    :repo  "emacs-eaf/emacs-application-framework"
                    :files ("*")))))

in packages.el

Maybe the eaf.el is also different, currently the eaf.el config several load-path

lesliebinbin · 2021-08-21T07:29:36Z

But if we remove these, the macos user might get errors, since the spacemacs in macos still use the eaf release in May

Is it possible to update the release?

On the other hand, we're actually planning to do real releases in the near future, after things are kinda more settled.

I will test a bit on macos, use this as a custom layer first, the difference is that when the os is not linux, it will use pip alternatives and pyqt5 install from pip got issues for long time

lebensterben · 2021-08-21T07:38:35Z

@lesliebinbin
Try removing eaf* from ~/.emacs.d/elpa/27.2/develop/ first

lesliebinbin · 2021-08-21T07:48:36Z

@lesliebinbin
Try removing eaf* from ~/.emacs.d/elpa/27.2/develop/ first

Test it on ubuntu 20,

lesliebinbin · 2021-08-21T08:04:45Z

@lesliebinbin
Try removing eaf* from ~/.emacs.d/elpa/27.2/develop/ first

Oh sorry, I may find some different, in my init.el file for spacemacs I use babel load and it requires the line (require 'eaf), is that makes the diffrence, if that is the case, is that a way to eager load eaf in packages.el?

lesliebinbin · 2021-08-21T08:12:30Z

Using the snippet from https://github.com/emacs-eaf/emacs-application-framework#1-download-eaf, won't correctly install eaf:
(quelpa '(eaf :fetcher github
              :repo  "emacs-eaf/emacs-application-framework"
              :files ("*")))
It shows
Compiling file /home/lucius/.emacs.d/elpa/27.2/develop/eaf-20210821.113/eaf.el at Sat Aug 21 01:34:44 2021
Entering directory ‘/home/lucius/.emacs.d/elpa/27.2/develop/eaf-20210821.113/’
eaf.el:167:1:Error: Cannot open load file: No such file or directory, eaf-epc
Leaving directory ‘/home/lucius/.emacs.d/elpa/27.2/develop/eaf-20210821.113/’
Ouch, this quelpa recipe is from the pre-split period not so long ago, we haven't updated it, I've removed from the README until a working recipe is emerged. But I only use use-package, not so sure what's the best way to write up a recipe at the moment, it is still an ongoing discussion.

The eaf-epc.el is located in eaf/core directory, do you know why isn't it loaded in the load-path

I thinks it is because we use defer t in packages.el for eaf, that disables the autoloads facility for eaf, I remove the defer

lebensterben · 2021-08-21T08:20:49Z

you're mixing things altogether.

quelpa downloads the package and install it. use-package loads the package.

Not related

lebensterben · 2021-08-23T23:36:25Z

@lesliebinbin
The "do not merge" label simply means there's a hard error, which I described in earlier comments.
It's set to notify other maintainers that at current stage merging this PR causes issues.

It doesn't mean this PR is out of consideration.

dalanicolai · 2021-09-01T10:34:01Z

But if we remove these, the macos user might get errors, since the spacemacs in macos still use the eaf release in May

@lesliebinbin I am not sure how Spacemacs works on macos, but I guess you are
using the same configuration as anyone else. So probably you have to update the
packages from the Spacemacs startup page (or press SPC f e U)

dalanicolai · 2021-09-01T10:55:12Z

@lesliebinbin Thanks for the work. The PR looks almost good to me.
@lebensterben @MatthewZMD Also thanks for your work guys.

I have tested this PR on a clean Spacemacs install, and it seems to work fine on
Fedora (after applying a small fix to the install-eaf.el script
emacs-eaf/emacs-application-framework#841).

dalanicolai · 2021-09-01T10:38:21Z

layers/+tools/eaf/README.org

-in their terminal. If the script does not work for you then [[https://github.com/manateelazycat/emacs-application-framework#dependency-list][install the
-required dependencies manually]].
+Currently the command =SPC SPC eaf-install-dependencies= no longer works, please
+refer to [[https://github.com/emacs-eaf/emacs-application-framework#2-installupdate-eaf-applications-and-dependencies][install the required dependencies manually]].


The new commands eaf-install and eaf-install-and-update work perfectly fine

dalanicolai · 2021-09-01T10:41:42Z

layers/+tools/eaf/README.org


 * Usage
+** Configuring applications
+By default, all applications provided by eaf package are loaded, including:


It is not necessary to remove applications from here, because you have added the NOERROR flag to the require statement, so Emacs will simply skip loading if some package is not available.

Still, this variable is useful for users who would like to load only some of the packages by default.

(You could add this explanation here)

dalanicolai · 2021-09-01T10:45:45Z

layers/+tools/eaf/README.org

+- eaf-camera
+
+To customise applications, please set the variable =eaf-apps=
+
 ** Configuring key bindings
 Configuration of EAF key bindings works in a different way than normally in Spacemacs.


As mentioned by @MatthewZMD, some keybindings might be added only conditionally. As checking all these bindings is a lot of work, you could simply add a warning about it. And add a request to report keybinding issues.

dalanicolai · 2021-09-01T10:48:30Z

layers/+tools/eaf/packages.el

@@ -20,15 +20,13 @@
 ;; You should have received a copy of the GNU General Public License
 ;; along with this program.  If not, see <http://www.gnu.org/licenses/>.

-
 (defconst eaf-packages
  '(ctable


Please remove the ctable, deferred and epc packages declarations.

dalanicolai · 2021-09-01T10:49:31Z

layers/+tools/eaf/packages.el

@@ -40,16 +38,12 @@
 (defun eaf/init-epc ()


Remove the ctable, deferred and epc init-functions

dalanicolai · 2021-09-01T11:11:15Z

Using the snippet from https://github.com/emacs-eaf/emacs-application-framework#1-download-eaf, won't correctly install eaf:

(quelpa '(eaf :fetcher github
              :repo  "emacs-eaf/emacs-application-framework"
              :files ("*")))

It shows

Compiling file /home/lucius/.emacs.d/elpa/27.2/develop/eaf-20210821.113/eaf.el at Sat Aug 21 01:34:44 2021
Entering directory ‘/home/lucius/.emacs.d/elpa/27.2/develop/eaf-20210821.113/’
eaf.el:167:1:Error: Cannot open load file: No such file or directory, eaf-epc
Leaving directory ‘/home/lucius/.emacs.d/elpa/27.2/develop/eaf-20210821.113/’

I think you can simply change this to:

(quelpa '(eaf :fetcher github
              :repo  "emacs-eaf/emacs-application-framework"
              :files ("*" "core/*.el")))

@MatthewZMD You should simply add the files and locations required for building the package (see here).

lebensterben · 2021-09-01T20:30:32Z

yes this works.

(quelpa '(eaf :fetcher github
              :repo  "emacs-eaf/emacs-application-framework"
              :files ("*" "core/*.el")))

MatthewZMD · 2021-09-02T01:41:08Z

          :files ("*" "core/*.el")))

I think extension/*.el will also be needed.

Since it works for you, could you please check this discussion which reported on straight, I want to know does quelpa have the same behavior - which copies the contents in the app directory to a new destination?

lebensterben · 2021-09-02T01:48:39Z

quelpa clone the whole repository.
that thread is too long and I don't want to read.

dalanicolai · 2021-09-02T06:36:41Z

          :files ("*" "core/*.el")))
I think extension/*.el will also be needed.

Since it works for you, could you please check this discussion which reported on straight, I want to know does quelpa have the same behavior - which copies the contents in the app directory to a new destination?

Quelpa builds the package in the same directory as the source. But I had a look at the straight problem. I will comment in that discussion.

* checkversion/develop: (55 commits) [doc] use org-mode example block for example Added COPYRIGHT file [ivy] Fix new src block in docs [ivy] counsel search commands use spacemacs--ivy-file-actions [core] Support packing elisp to *.elc and *.el.gz files [version-control] Switch default version control diff tool to git-gutter [python] Remove comments and fix new commands [bot] "built_in_updates" Wed Sep 29 19:18:45 UTC 2021 Add pydoc to python layer for better python doc functionality [python] Make ipython version detection more robust Revise some smaller layers [bot] "documentation_updates" Fri Sep 17 19:24:14 UTC 2021 Fix (activate) company-emoji completion [bot] "documentation_updates" Fri Sep 17 18:59:01 UTC 2021 Add documentation about evilifying 'special-mode buffers' (+fix eww) Fix and update PR syl20bnr#14992: update eaf layer Update keybinding Extend spacemacs/copy-file command with extra actions (syl20bnr#14754) Add simple workaround to ebib kill buffer issue (syl20bnr#15048) [bot] "documentation_updates" Sat Sep 11 22:58:39 UTC 2021 ...

dalanicolai mentioned this pull request Aug 16, 2021

Installation via M-x eaf-install-dependencies does not work well emacs-eaf/emacs-application-framework#633

Closed

lebensterben reviewed Aug 16, 2021

View reviewed changes

layers/+tools/eaf/packages.el Outdated Show resolved Hide resolved

lebensterben approved these changes Aug 16, 2021

View reviewed changes

MatthewZMD reviewed Aug 16, 2021

View reviewed changes

lebensterben self-requested a review August 20, 2021 03:17

lebensterben requested changes Aug 20, 2021

View reviewed changes

lesliebinbin requested a review from lebensterben August 21, 2021 04:45

lebensterben requested changes Aug 21, 2021

View reviewed changes

lebensterben reviewed Aug 21, 2021

View reviewed changes

lesliebinbin added 10 commits August 21, 2021 16:37

fix leader key for eaf camera

ea6cabb

load eaf app packages without causing errors

c709a99

modify based on review

b632b5e

remove redundant packages

eb05dcd

move to :config section

37b1ec4

fix and optimise config.el

101f64c

add description to doc

f4ad491

update links

897b356

restore ctable deferred epc to fix broken install

2fa531f

update link for manually install

409b4fe

MatthewZMD reviewed Aug 21, 2021

View reviewed changes

fix typo

9ece119

remove defer to use auto-loads provided by eaf

e2bc106

lebensterben added the DO NOT MERGE label Aug 23, 2021

lesliebinbin closed this Aug 23, 2021

dalanicolai reviewed Sep 1, 2021

View reviewed changes

dalanicolai added a commit to dalanicolai/spacemacs that referenced this pull request Sep 17, 2021

Fix and update PR syl20bnr#14992: update eaf layer

ca48c20

dalanicolai mentioned this pull request Sep 17, 2021

Fix and update PR #14992: update eaf layer #15051

Closed

smile13241324 pushed a commit that referenced this pull request Sep 17, 2021

Fix and update PR #14992: update eaf layer

c8c20be

2ynn pushed a commit to 2ynn/spacemacs that referenced this pull request Jun 22, 2022

Fix and update PR syl20bnr#14992: update eaf layer

74b4222

Adjust eaf layer to newer eaf package #14992

Adjust eaf layer to newer eaf package #14992

Conversation

lesliebinbin commented Aug 15, 2021

dalanicolai commented Aug 16, 2021 • edited

lebensterben left a comment

Choose a reason for hiding this comment

MatthewZMD Aug 16, 2021 • edited

Choose a reason for hiding this comment

MatthewZMD commented Aug 16, 2021 • edited

lebensterben left a comment

Choose a reason for hiding this comment

lesliebinbin commented Aug 21, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lebensterben commented Aug 21, 2021

Choose a reason for hiding this comment

lebensterben commented Aug 21, 2021

MatthewZMD commented Aug 21, 2021

MatthewZMD commented Aug 21, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lesliebinbin commented Aug 21, 2021 • edited

lesliebinbin commented Aug 21, 2021

lebensterben commented Aug 21, 2021

lesliebinbin commented Aug 21, 2021

lesliebinbin commented Aug 21, 2021

lesliebinbin commented Aug 21, 2021

lebensterben commented Aug 21, 2021

lebensterben commented Aug 23, 2021

dalanicolai commented Sep 1, 2021

dalanicolai commented Sep 1, 2021

dalanicolai Sep 1, 2021 • edited

Choose a reason for hiding this comment

dalanicolai Sep 1, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dalanicolai commented Sep 1, 2021 • edited

lebensterben commented Sep 1, 2021

MatthewZMD commented Sep 2, 2021

lebensterben commented Sep 2, 2021

dalanicolai commented Sep 2, 2021

dalanicolai commented Aug 16, 2021 •

edited

MatthewZMD Aug 16, 2021 •

edited

MatthewZMD commented Aug 16, 2021 •

edited

lesliebinbin commented Aug 21, 2021 •

edited

MatthewZMD commented Aug 21, 2021 •

edited

lesliebinbin commented Aug 21, 2021 •

edited

dalanicolai Sep 1, 2021 •

edited

dalanicolai Sep 1, 2021 •

edited

dalanicolai commented Sep 1, 2021 •

edited