Hello Philip, Philip Kaludercic writes: > Thierry Volpiatto writes: > >> Philip Kaludercic writes: >> >>> Thierry Volpiatto writes: >>> >>>> I see that Helm installation from nongnu elpa doesn't include >>>> emacs-helm.sh script, would it be possible to modify this? >>>> >>>> diff --git a/elpa-packages b/elpa-packages >>>> index 77d7a5409e..0923724b62 100644 >>>> --- a/elpa-packages >>>> +++ b/elpa-packages >>>> @@ -312,11 +312,11 @@ >>>> :ignored-files "COPYING") >>>> >>>> (helm :url "https://github.com/emacs-helm/helm" >>>> - :ignored-files ("images" "Cask" "Makefile" "emacs-helm.sh" "helm-core.el" "helm.el" >>>> + :ignored-files ("images" "Makefile" "helm-core.el" "helm.el" >>>> "helm-lib.el" "helm-source.el" "helm-multi-match.el")) >>>> >>>> (helm-core :url "https://github.com/emacs-helm/helm" >>>> - :ignored-files ("images" "Cask" "Makefile" "emacs-helm.sh" "helm-adaptive.el" >>>> + :ignored-files ("images" "Makefile" "emacs-helm.sh" "helm-adaptive.el" >>>> "helm-bookmark.el" "helm-buffers.el" "helm-color.el" "helm-comint.el" >>>> "helm-command.el" "helm-config.el" "helm-dabbrev.el" "helm-easymenu.el" >>>> "helm-elisp.el" "helm-elisp-package.el" "helm-epa.el" "helm-eshell.el" >>> >>> There shouldn't be any issue in applying this patch, but could you >>> explain the propose of this script? >> >> It is used specially for reproducing bugs in a clean environment, see it >> as emacs -Q for Emacs when reporting bugs. This script starts Emacs -Q >> with only Helm loaded, this ensure the bug if one comes from Helm and >> not another package. This is important especially nowaday people are >> using "Emacs distribution" with the world list of packages installed. >> Apart that the script is useful to quickly launch Emacs with helm, one >> can use it from the Helm directory or symlinked to e.g. ~/bin. > > I see. In that case is there any reason you implement this as a shell > script? Well when I wrote the script, packages where not existing and from outside emacs it is actually the only way to run a package isolated. > (It might be interesting to provide something like this for > package.el, to test packages in a generic way.) Yes, this would be interesting, it would be something like this: Emacs -Q M-x > I am sort of an unofficial co-maintainer, without access to any > server-side infrastructure, but other than that I'm glad to help within > my possibilities. Ok thanks. >> Ok, it's what I thought, so yes it would be nice to install wfnames >> package in Nongnu. >> Here is the link: https://github.com/thierryvolpiatto/wfnames > > Sure, but could you first take a look at these comments, suggestions and > questions I have made while skimming over the source? > > diff --git a/wfnames.el b/wfnames.el > index e5a83b5..a61a18d 100644 > --- a/wfnames.el > +++ b/wfnames.el > @@ -34,20 +34,20 @@ > > ;; Usage: > ;; Once in the Wfnames buffer, edit your filenames and hit C-c C-c to > -;; save your changes. You have completion on filenames and directories > +;; save your changes. You have completion on filenames and directories > ;; with TAB but if you are using Iedit package and it is in action use =M-TAB=. > > ;;; Code: > > -(require 'cl-lib) > +(eval-when-compile (require 'cl-lib)) ;you are only using macros, so this should be OK Ok done. > ;; Internal. > -(defvar wfnames-buffer "*Wfnames*") > -(defvar wfnames--modified nil) > +(defvar wfnames-buffer "*Wfnames*") ;perhaps `defconst' > +(defvar-local wfnames--modified nil) Ok done. > > (defgroup wfnames nil > "A mode to edit filenames." > - :group 'wfnames) > + :group 'wfnames) ;watch out, this is a recursive group Good catch, done. > (defcustom wfnames-create-parent-directories t > "Create parent directories when non nil." > @@ -57,16 +57,21 @@ > "Ask confirmation when overwriting." > :type 'boolean) > > -(defvar wfnames-after-commit-hook nil) > +(defcustom wfnames-after-commit-hook nil > + "Hook to run after `wfnames-commit-buffer'." ;rephrase this Not sure how to rephrase this: "Hook that run after `wfnames-commit-buffer'."? > > + :type 'hook) Othewise defcustom make sense, done. > (defcustom wfnames-after-commit-function #'kill-buffer > "A function to call on `wfnames-buffer' when done." > :type 'function) > > (defcustom wfnames-make-backup nil > - "Backup files before overwriting when non nil." > + "Non-nil means files are backed up before overwriting." Ok done. > > :type 'boolean) > > +;; instead of defining new faces and colours, do you think it would be > +;; possible to inherit from existing faces? Apart requiring a package just for the faces no, font-lock family doesn't provide :background faces. > (defface wfnames-modified > '((t :background "LightBlue" :foreground "black")) > "Face used when filename is modified.") > @@ -104,27 +109,27 @@ > "Provide filename completion in wfnames buffer." > (let ((beg (line-beginning-position)) > (end (point))) > + ;; Does it make sense to extend beyond END to allow completing > + ;; file names mid-string? This one for now I don't know, I transformed your comment with a FIXME. > > (list beg end #'completion-file-name-table :exit-function (lambda > (str _status) (when (and (stringp str) (eq (char-after) ?/)) > (delete-char -1)))))) > > -(define-derived-mode wfnames-mode > - text-mode "wfnames" > +(define-derived-mode wfnames-mode text-mode "wfnames" > "Major mode to edit filenames. Ok. > Special commands: > \\{wfnames-mode-map}" > (add-hook 'after-change-functions #'wfnames-after-change-hook nil t) > - (make-local-variable 'wfnames--modified) > - (set (make-local-variable 'completion-at-point-functions) #'wfnames-capf) > - (set (make-local-variable 'revert-buffer-function) #'wfnames-revert-changes)) > + (setq-local completion-at-point-functions #'wfnames-capf) > + (setq-local revert-buffer-function #'wfnames-revert-changes)) Yes done. > (defun wfnames-abort () > "Quit and kill wfnames buffer." > (interactive) > - (quit-window t)) > + (quit-window t)) ;isn't this `kill-buffer-and-window' Yes, but quit-window is fine as well (I am used to it). > (defun wfnames-after-change-hook (beg end _len) > "Put overlay on current line when modified. > @@ -137,9 +142,10 @@ Args BEG and END delimit changes on line." > (eol (line-end-position)) > (old (get-text-property bol 'old-name)) > (new (buffer-substring-no-properties bol eol)) > - ov face) > - (setq face (if (file-exists-p new) > - 'wfnames-modified-exists 'wfnames-modified)) > + (face (if (file-exists-p new) > + 'wfnames-modified-exists > + 'wfnames-modified)) > + ov) Ok. > (setq-local wfnames--modified > (cons old (delete old wfnames--modified))) > (cl-loop for o in (overlays-in bol eol) > @@ -184,10 +190,10 @@ When APPEND is specified, append FILES to existing `wfnames-buffer'." > "* " > 'face 'wfnames-prefix)) > "\n")) > - (when append (delete-duplicate-lines (point-min) (point-max)))) > + (when append (delete-duplicate-lines (point-min) (point-max)))) ;this requires Emacs 24.4 > (unless append > ;; Go to beginning of basename on first line. > - (while (re-search-forward "/" (line-end-position) t)) > + (re-search-forward "\\(?:/[^/]*\\)*/" (line-end-position) t) Ok. > (wfnames-mode) > (funcall display-fn wfnames-buffer)))) > > @@ -202,7 +208,7 @@ When APPEND is specified, append FILES to existing `wfnames-buffer'." > "Backup FILE." > (when wfnames-make-backup > (with-current-buffer (find-file-noselect file) > - (let ((backup-by-copying t)) > + (let ((backup-by-copying t)) ;why is this bound? isn't this > a user preference? Yes removed. > > (backup-buffer)) (kill-buffer)))) > > @@ -252,7 +258,7 @@ When APPEND is specified, append FILES to existing `wfnames-buffer'." > (let ((basedir (file-name-directory > (directory-file-name new)))) > (unless (file-directory-p basedir) > - (mkdir basedir 'parents)))) > + (make-directory basedir > 'parents)))) Ok, make sense. > (if (and ow (wfnames-ask-for-overwrite new)) > ;; Direct overwrite i.e. first loop. > (progn > @@ -300,21 +306,22 @@ With a numeric prefix ARG, revert the ARG next lines." > (wfnames-revert-current-line-1) > (when (eobp) (forward-line -1)) > (goto-char (line-beginning-position)) > - (while (re-search-forward "/" (line-end-position) t)))) > + (re-search-forward "\\(?:/[^/]*\\)*/" (line-end-position) t))) Ok, same as above. > (defun wfnames-revert-changes (_ignore-auto _no-confirm) > "Revert wfnames buffer to its initial state. > > This is used as `revert-buffer-function' for `wfnames-mode'." > (with-current-buffer wfnames-buffer > - (cl-loop for o in (overlays-in (point-min) (point-max)) > - when (overlay-get o 'hff-changed) > - do (delete-overlay o)) > + (dolist (o (overlays-in (point-min) (point-max))) > + (when (overlay-get o 'hff-changed) > + (delete-overlay o))) If you want ;-) done. > (goto-char (point-min)) > (save-excursion > (while (not (eobp)) > (wfnames-revert-current-line-1))) > - (while (re-search-forward "/" (line-end-position) t)))) > + (re-search-forward "\\(?:/[^/]*\\)*/" (line-end-position) t))) > + Ok same as above. > (provide 'wfnames) > > > >> Thanks. Thanks. -- Thierry