Okay, I will improve your Emacs configuration. Here's what I'll do: #9
Conversation
mikefaille
commented
Jul 17, 2025
mikefaille
commented
- Add error handling to package installation
- Optimize for lazy loading to improve startup time
- Add documentation to all package configurations
- Add error handling to package installation - Optimize for lazy loading to improve startup time - Add documentation to all package configurations
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @mikefaille, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces significant improvements to the Emacs configuration, primarily focusing on optimizing startup performance through widespread lazy loading of packages. It also enhances the reliability of package installation with better error handling and improves the overall maintainability and readability of the configuration by centralizing package loading and adding comprehensive documentation.
Highlights
- Enhanced Package Installation: The
require-packagefunction incore/core-packages.elhas been refactored to explicitly refresh package contents and install packages with robust error handling, ensuring more reliable package management. - Optimized Lazy Loading: A significant portion of the Emacs configuration, particularly within the
packages/directory, has been updated to utilizeuse-packagewith the:defer tkeyword. This change ensures that packages are loaded only when needed, contributing to a faster Emacs startup time. - Centralized Package Loading: A new file,
packages/pkg-loader.el, has been introduced to centralize all individual packagerequirestatements. The maininit.elnow simply requirespkg-loader, streamlining the initial loading process and making the configuration more modular. - Improved Documentation and Code Cleanup: All package configuration files (
packages/*.el) now include standard Emacs Lisp file headers with commentary and code sections. Numerous inline comments have been added to explain configurations, and deprecated or commented-out code has been removed across various files, enhancing overall code readability and maintainability.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The code changes improve the Emacs configuration by adding error handling to package installation, optimizing for lazy loading to improve startup time, and adding documentation to all package configurations. The changes also include configuration for various packages such as corfu, lsp, git, and more.
| (eshell-send-input) | ||
| (delete-window)) | ||
|
|
||
| (global-set-key (kbd "C-!") 'eshell-toggle) |
There was a problem hiding this comment.
The eshell-toggle function is being bound to C-! but is not defined in this block. It would be helpful to define it or remove the keybinding.
| (defun require-package (package) | ||
| "Ensure PACKAGE is installed." | ||
| "Ensure PACKAGE is installed, with error handling." | ||
| (unless (package-installed-p package) | ||
| (maybe-refresh-package-contents package) | ||
| (condition-case err | ||
| (eval `(use-package ,package | ||
| :ensure t)) | ||
| (progn | ||
| (package-refresh-contents) | ||
| (package-install package)) | ||
| (error (message "Failed to install %s: %S" package err))))) |
There was a problem hiding this comment.
Consider adding a comment to explain what the condition-case block is doing, specifically what errors it is catching and why. This will improve the readability of the code.
(condition-case err
(progn
(package-refresh-contents)
(package-install package))
(error (message "Failed to install %s: %S" package err))) ; Handle errors during package installation
| (setq cape-dabbrev-min-length 2 | ||
| cape-dabbrev-check-other-buffers 'some) | ||
|
|
||
| ;; Ensure Cape works well with Corfu | ||
| ;; Ensure Cape works well with Corfu. | ||
| (advice-add 'cape-wrap-buster :around 'cape-capf-buster) |
There was a problem hiding this comment.
The advice-add function modifies an existing function. It would be helpful to add a comment explaining what cape-wrap-buster and cape-capf-buster do, and why this advice is needed to ensure Cape works well with Corfu.
(advice-add 'cape-wrap-buster :around 'cape-capf-buster) ; Ensure Cape works well with Corfu
| @@ -37,11 +57,6 @@ | |||
|
|
|||
| ;; Highlight the current line | |||
| (global-hl-line-mode +1) | |||
There was a problem hiding this comment.
Consider adding a comment explaining why global-hl-line-mode is being enabled. This will improve the readability of the code.
(global-hl-line-mode +1) ; Highlight the current line
There was a problem hiding this comment.
Code Review
The pull request introduces a new package loader and refactors existing package configurations to use use-package more effectively, promoting lazy loading and better code organization. The review focuses on suggesting further improvements to modularity, code clarity, and consistency in package configurations.
| (use-package bash-completion | ||
| :defer t | ||
| :config | ||
| ;; Function to turn off indent-tabs-mode. | ||
| (defun turn-off-indent-tabs-mode () | ||
| "Turn off the use of tabs for indentation." | ||
| (setq indent-tabs-mode nil) | ||
| (setq tab-width 8)) | ||
|
|
||
| ;; Add hook for shell script mode | ||
| (add-hook 'sh-mode-hook 'turn-off-indent-tabs-mode) | ||
| ;; Add hook for shell script mode. | ||
| (add-hook 'sh-mode-hook 'turn-off-indent-tabs-mode) | ||
|
|
||
| ;; Function to generate auto-complete candidates for bash | ||
| (defun ac-bash-candidates () | ||
| "Generate a list of potential completions for the current bash command. | ||
| ;; Function to generate auto-complete candidates for bash. | ||
| (defun ac-bash-candidates () | ||
| "Generate a list of potential completions for the current bash command. | ||
| This function is a modified version of `bash-completion-dynamic-complete' | ||
| from bash-completion.el." | ||
| (when bash-completion-enabled | ||
| (let* ((start (comint-line-beginning-position)) | ||
| (pos (point)) | ||
| (tokens (bash-completion-tokenize start pos)) | ||
| (open-quote (bash-completion-tokenize-open-quote tokens)) | ||
| (parsed (bash-completion-process-tokens tokens pos)) | ||
| (line (cdr (assq 'line parsed))) | ||
| (point (cdr (assq 'point parsed))) | ||
| (cword (cdr (assq 'cword parsed))) | ||
| (words (cdr (assq 'words parsed))) | ||
| (stub (nth cword words)) | ||
| (completions (bash-completion-comm line point words cword open-quote)) | ||
| ;; Override configuration for comint-dynamic-simple-complete. | ||
| ;; Bash adds a space suffix automatically. | ||
| (comint-completion-addsuffix nil)) | ||
| completions))) | ||
| (when bash-completion-enabled | ||
| (let* ((start (comint-line-beginning-position)) | ||
| (pos (point)) | ||
| (tokens (bash-completion-tokenize start pos)) | ||
| (open-quote (bash-completion-tokenize-open-quote tokens)) | ||
| (parsed (bash-completion-process-tokens tokens pos)) | ||
| (line (cdr (assq 'line parsed))) | ||
| (point (cdr (assq 'point parsed))) | ||
| (cword (cdr (assq 'cword parsed))) | ||
| (words (cdr (assq 'words parsed))) | ||
| (stub (nth cword words)) | ||
| (completions (bash-completion-comm line point words cword open-quote)) | ||
| ;; Override configuration for comint-dynamic-simple-complete. | ||
| ;; Bash adds a space suffix automatically. | ||
| (comint-completion-addsuffix nil)) | ||
| completions)))) |
There was a problem hiding this comment.
The use-package macro should be used to configure bash-completion and define the turn-off-indent-tabs-mode and ac-bash-candidates functions within its :config block. This ensures that these functions are only defined when bash-completion is loaded, improving modularity and lazy loading.
(use-package bash-completion
:defer t
:config
;; Function to turn off indent-tabs-mode.
(defun turn-off-indent-tabs-mode ()
"Turn off the use of tabs for indentation."
(setq indent-tabs-mode nil)
(setq tab-width 8))
;; Add hook for shell script mode.
(add-hook 'sh-mode-hook 'turn-off-indent-tabs-mode)
;; Function to generate auto-complete candidates for bash.
(defun ac-bash-candidates ()
"Generate a list of potential completions for the current bash command.
This function is a modified version of `bash-completion-dynamic-complete'
from bash-completion.el."
(when bash-completion-enabled
(let* ((start (comint-line-beginning-position))
(pos (point))
(tokens (bash-completion-tokenize start pos))
(open-quote (bash-completion-tokenize-open-quote tokens))
(parsed (bash-completion-process-tokens tokens pos))
(line (cdr (assq 'line parsed)))
(point (cdr (assq 'point parsed)))
(cword (cdr (assq 'cword parsed)))
(words (cdr (assq 'words parsed)))
(stub (nth cword words))
(completions (bash-completion-comm line point words cword open-quote))
;; Override configuration for comint-dynamic-simple-complete.
;; Bash adds a space suffix automatically.
(comint-completion-addsuffix nil))
completions))))
| (setq completion-at-point-functions | ||
| (list | ||
| (cape-capf-super | ||
| #'cape-file | ||
| #'cape-dabbrev | ||
| #'cape-abbrev ; Changed from cape-keyword | ||
| #'cape-line | ||
| #'cape-dict))) | ||
|
|
||
| ;; Configure Cape settings | ||
| (list | ||
| (cape-capf-super | ||
| #'cape-file | ||
| #'cape-dabbrev | ||
| #'cape-abbrev | ||
| #'cape-line | ||
| #'cape-dict))) |
There was a problem hiding this comment.
Consider using use-package's :init keyword to set completion-at-point-functions only when cape is loaded. This ensures that the completion functions are set up correctly and avoids potential issues if cape is not available.
(use-package cape
:defer t
:init
(setq completion-at-point-functions
(list
(cape-capf-super
#'cape-file
#'cape-dabbrev
#'cape-abbrev
#'cape-line
#'cape-dict))))
| (setq lsp-dart-sdk-dir "/opt/flutter/") | ||
| (add-to-list 'exec-path (concat lsp-dart-sdk-dir "bin/")) |
There was a problem hiding this comment.
Consider defining lsp-dart-sdk-dir and adding it to exec-path within the :config block of use-package lsp-dart. This ensures that these settings are applied only when lsp-dart is loaded, improving modularity and lazy loading.
(use-package lsp-dart
:defer t
:hook (dart-mode . lsp)
:config
(setq lsp-dart-sdk-dir "/opt/flutter/")
(add-to-list 'exec-path (concat lsp-dart-sdk-dir "bin/")))
| ;; Override M-x to use Ido for command selection. | ||
| (global-set-key | ||
| "\M-x" | ||
| (lambda () | ||
| (interactive) | ||
| (call-interactively | ||
| (intern | ||
| (ido-completing-read | ||
| "M-x " | ||
| (all-completions "" obarray 'commandp))))))) |
There was a problem hiding this comment.
Consider using use-package's :config block to define the lambda function that overrides M-x. This ensures that the key binding is set only when ido is loaded, improving modularity and lazy loading.
(use-package ido
:init
;; Enable Ido mode for all buffer and file operations.
(ido-mode 1)
(ido-everywhere 1)
;; Enable flexible matching and use faces in Ido.
(setq ido-enable-flex-matching t)
(setq ido-use-faces t)
:config
;; Override M-x to use Ido for command selection.
(global-set-key
"\M-x"
(lambda ()
(interactive)
(call-interactively
(intern
(ido-completing-read
"M-x "
(all-completions "" obarray 'commandp)))))))
| (add-hook 'org-mode-hook | ||
| (lambda () | ||
| ;; Use bullets for headlines. | ||
| (org-bullets-mode) | ||
| ;; Use visual line mode for better readability. | ||
| (visual-line-mode t) | ||
| ;; Fontify source blocks natively. | ||
| (setq-default org-src-fontify-natively t))) |
There was a problem hiding this comment.
Consider using :hook to set org-bullets-mode, visual-line-mode, and org-src-fontify-natively within org-mode-hook. This ensures that these settings are applied only when org-mode is loaded, improving modularity and lazy loading.
(use-package org
:defer t
:hook (org-mode . (lambda ()
(org-bullets-mode)
(visual-line-mode t)
(setq-default org-src-fontify-natively t))))
| (autoload 'ssh-config-mode "ssh-config-mode" t) | ||
| ;; Associate ssh config files with ssh-config-mode. | ||
| (add-to-list 'auto-mode-alist '(".ssh/config\\'" . ssh-config-mode)) | ||
| (add-to-list 'auto-mode-alist '("sshd?_config\\'" . ssh-config-mode)) | ||
| (add-to-list 'auto-mode-alist '("known_hosts\\'" . ssh-known-hosts-mode)) | ||
| (add-to-list 'auto-mode-alist '("authorized_keys2?\\'" . ssh-authorized-keys-mode)) | ||
| (add-hook 'ssh-config-mode-hook 'turn-on-font-lock) | ||
|
|
||
| (setq tramp-chunksize 1300) | ||
| ;; Enable font lock in ssh-config-mode. | ||
| (add-hook 'ssh-config-mode-hook 'turn-on-font-lock) | ||
| ;; Set the tramp chunksize to improve performance over slow connections. | ||
| (setq tramp-chunksize 1300)) |
There was a problem hiding this comment.
Consider using :config block to define the autoload and add-to-list functions within use-package ssh-config-mode. This ensures that these settings are applied only when ssh-config-mode is loaded, improving modularity and lazy loading.
(use-package ssh-config-mode
:defer t
:config
;; Autoload ssh-config-mode.
(autoload 'ssh-config-mode "ssh-config-mode" t)
;; Associate ssh config files with ssh-config-mode
(add-to-list 'auto-mode-alist '(".ssh/config\'" . ssh-config-mode))
(add-to-list 'auto-mode-alist '("sshd?_config\'" . ssh-config-mode))
(add-to-list 'auto-mode-alist '("known_hosts\'" . ssh-known-hosts-mode))
(add-to-list 'auto-mode-alist '("authorized_keys2?\'" . ssh-authorized-keys-mode))
;; Enable font lock in ssh-config-mode.
(add-hook 'ssh-config-mode-hook 'turn-on-font-lock)
;; Set the tramp chunksize to improve performance over slow connections.
(setq tramp-chunksize 1300))
| (add-hook 'web-mode-hook | ||
| (lambda () | ||
| ;; Disable whitespace-mode in web-mode. | ||
| (whitespace-mode -1) | ||
| ;; Enable emmet-mode for sgml-mode. | ||
| (with-eval-after-load 'sgml-mode | ||
| (use-package emmet-mode | ||
| :defer t | ||
| :config | ||
| (emmet-mode 1))) | ||
| ;; Enable emmet-mode for css-mode. | ||
| (with-eval-after-load 'css-mode | ||
| (use-package emmet-mode | ||
| :defer t | ||
| :config | ||
| (emmet-mode 1))) | ||
| ;; Enable css-eldoc for web-mode. | ||
| (with-eval-after-load 'web-mode | ||
| (use-package css-eldoc | ||
| :defer t))))) |
There was a problem hiding this comment.
Consider using use-package's :hook keyword to add the lambda function to web-mode-hook. This ensures that the hook is added only when web-mode is loaded, improving modularity and lazy loading.
(use-package web-mode
:defer t
:mode (("\\.html?\\'" . web-mode)
("\\.phtml\'" . web-mode)
("\\.tpl\\.php\'" . web-mode)
("\\.[agj]sp\'" . web-mode)
("\\.as[cp]x\'" . web-mode)
("\\.erb\'" . web-mode)
("\\.mustache\'" . web-mode)
("\\.djhtml\'" . web-mode)
("\\.css\'" . css-mode))
:config
;; Define the web mode engines.
(setq web-mode-engines-alist
'(("php" . "\\.phtml\'")
("blade" . "\\.blade\\.")))
:hook (web-mode . (lambda ()
;; Disable whitespace-mode in web-mode.
(whitespace-mode -1)
;; Enable emmet-mode for sgml-mode.
(with-eval-after-load 'sgml-mode
(use-package emmet-mode
:defer t
:config
(emmet-mode 1)))
;; Enable emmet-mode for css-mode.
(with-eval-after-load 'css-mode
(use-package emmet-mode
:defer t
:config
(emmet-mode 1)))
;; Enable css-eldoc for web-mode.
(with-eval-after-load 'web-mode
(use-package css-eldoc
:defer t)))))
| (add-hook 'yaml-mode-hook | ||
| (lambda () | ||
| (local-set-key (kbd "C-c >") 'indent-tools-hydra/body) | ||
| (eval-after-load 'flycheck | ||
| '(add-hook 'flycheck-mode-hook 'flycheck-yamllint-setup))))) |
There was a problem hiding this comment.
Consider using :hook to add the lambda function to yaml-mode-hook. This ensures that the hook is added only when yaml-mode is loaded, improving modularity and lazy loading.
(use-package yaml-mode
:defer t
:mode (("\\.yml$" . yaml-mode)
("\\.yaml$" . yaml-mode))
:config
(require-package 'indent-tools)
(use-package indent-tools
:defer t)
(require-package 'flycheck-yamllint)
(use-package flycheck-yamllint
:defer t)
:hook (yaml-mode . (lambda ()
(local-set-key (kbd "C-c >") 'indent-tools-hydra/body)
(eval-after-load 'flycheck
'(add-hook 'flycheck-mode-hook 'flycheck-yamllint-setup)))))
