Re: Partial wdired (edit just filename at the point)

[Stefan Monnier]

Hi,

These changes are looking quite good.  There are a few things to fix
before we can install it, tho:

> diff --git a/lisp/wdired.el b/lisp/wdired.el
> index c495d8de34..ba71306e66 100644
> --- a/lisp/wdired.el
> +++ b/lisp/wdired.el
> @@ -250,20 +250,11 @@ wdired-change-to-wdired-mode
>    (setq buffer-read-only nil)
>    (dired-unadvertise default-directory)
>    (add-hook 'kill-buffer-hook 'wdired-check-kill-buffer nil t)
> +  (add-hook 'before-change-functions 'wdired--preprocess-line nil t)
>    (add-hook 'after-change-functions 'wdired--restore-properties nil t)

[ Nitpick:
  Please use #' rather than ' to quote functions (I know the rest of the
  code here doesn't (yet), but we should move towards more systematic
  use of #' so as to better catch obsolete functions and other issues).  ]

> +(defun wdired--preprocess-line (beg end)
> +  "Preprocess current line under point to make it writable.  "

This is incorrect: what it *should* do is to preprocess an area that
covers BEG..END.  That's why I suggested to name it "...lines" rather
than "...line" ;-)
And the "current point" is not really relevant (it will *usually* be in
the vicinity, but not always).

> +  (let ((inhibit-read-only t))
> +    (unless (get-text-property (line-beginning-position) 'front-sticky)
> +      (buffer-disable-undo)

As you can guess from the previous comment, we should loop over BEG...END
to process all the lines involved.

Also, I recommend you use `with-silent-modifications` which will cover
both `inhibit-read-only` and the undo (and a few more potential
problems, which admittedly should hopefully not matter here).
[ That macro didn't exist back when wdired.el was written.  ]

> @@ -304,9 +303,51 @@ wdired-preprocess-files
>                         (looking-at "l")
>                         (search-forward " -> " (line-end-position) t)))
>              (goto-char (line-end-position)))
> -       (setq b-protection (point))
> -        (forward-line))
> -      (put-text-property b-protection (point-max) 'read-only t))))
> +          (setq b-protection (point))
> +          (put-text-property b-protection (line-end-position)
> +                             'read-only t))
> +
> +        ;; Put a keymap property to the permission bits of the files,
> +        ;; and store the original name and permissions as a property
> +        (when wdired-allow-to-change-permissions
> +          (goto-char (line-beginning-position))
> +          (setq-local wdired-col-perm nil)
> +          (when (and (not (looking-at dired-re-sym))
> +                  (wdired-get-filename)
> +                  (re-search-forward dired-re-perms
> +                                        (line-end-position) 'eol))
> +         (let ((begin (match-beginning 0))
> +               (end (match-end 0)))
> +           (unless wdired-col-perm
> +             (setq wdired-col-perm (- (current-column) 9)))
> +           (if (eq wdired-allow-to-change-permissions 'advanced)
> +               (progn
> +                 (put-text-property begin end 'read-only nil)
> +                 ;; make first permission bit writable
> +                 (put-text-property
> +                  (1- begin) begin 'rear-nonsticky '(read-only)))
> +             ;; avoid that keymap applies to text following permissions
> +             (add-text-properties
> +              (1+ begin) end
> +              `(keymap ,wdired-perm-mode-map rear-nonsticky (keymap))))
> +           (put-text-property end (1+ end) 'end-perm t)
> +           (put-text-property
> +            begin (1+ begin) 'old-perm (match-string-no-properties 0)))))
> +
> +        ;; Put the needed properties to allow the user to change
> +        ;; links' targets
> +        (when (fboundp 'make-symbolic-link)
> +          (goto-char (line-beginning-position))
> +          (when (looking-at dired-re-sym)
> +            (re-search-forward " -> \\(.*\\)$")
> +         (put-text-property (1- (match-beginning 1))
> +                            (match-beginning 1) 'old-link
> +                            (match-string-no-properties 1))
> +            (put-text-property (match-end 1) (1+ (match-end 1)) 'end-link t)
> +            (unless wdired-allow-to-redirect-links
> +              (put-text-property (match-beginning 0)
> +                              (match-end 1) 'read-only t))))))
> +    (buffer-enable-undo)))

To minimize code changes (and to avoid making such a large function
(which are sadly frequent in Emacs's code base)), I think you can keep
the `wdired-preprocess-perms` and `wdired-preprocess-symlinks` (tho
you'll want to change them by making them take the beg..end arguments.)
and call them from here instead of moving their code into this new
function.


        Stefan