From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Thierry Volpiatto Newsgroups: gmane.emacs.devel Subject: Re: Partial wdired (edit just filename at the point) Date: Thu, 18 Mar 2021 11:32:41 +0100 Message-ID: <87a6r04u94.fsf@posteo.net> References: Mime-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="28004"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: mu4e 1.5.10; emacs 28.0.50 Cc: Stefan Monnier , emacs-devel@gnu.org To: Arthur Miller Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Thu Mar 18 11:44:26 2021 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lMq8g-0007Ac-7U for ged-emacs-devel@m.gmane-mx.org; Thu, 18 Mar 2021 11:44:26 +0100 Original-Received: from localhost ([::1]:41926 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lMq8f-0000Em-3S for ged-emacs-devel@m.gmane-mx.org; Thu, 18 Mar 2021 06:44:25 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:60814) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lMq82-0008GL-Ho for emacs-devel@gnu.org; Thu, 18 Mar 2021 06:43:48 -0400 Original-Received: from mout01.posteo.de ([185.67.36.65]:36489) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lMq7y-0004a5-9u for emacs-devel@gnu.org; Thu, 18 Mar 2021 06:43:46 -0400 Original-Received: from submission (posteo.de [89.146.220.130]) by mout01.posteo.de (Postfix) with ESMTPS id 19641160063 for ; Thu, 18 Mar 2021 11:43:38 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017; t=1616064219; bh=DmKNV2FS3E/8bpHyB9LCfrlhle81zKrTTM+Xv21AO6o=; h=From:To:Cc:Subject:Date:Autocrypt:From; b=enqoHlc2utoud94lvC0cHBr6rGvRfe0Rqd24QXqX1xvvBxdMBa/gutmYT2LbvhVpf xbdp/coPba9EByolAlOq/Wf0nsGufyV9ZCYcRID+/M27MoMIusmBnzdIMS0EbLcllU J0B/zl7UxbJkgh78jW2RiHQ7m2IDZZpkOFFZDV3C7j9nvkQltVGSuGLRoycY9z1DHZ nBsTVNuLCRaD/Xdo79u/wTNJrCynSi0CyP3D6rcxVushwA41lTJ74JnZNPa2W1eEN3 qkY18xhKvC8w2VFJChRO18uvWm5/IdwNFl+4VObdGymgZl/OB2jImRVexD6Q3yx9yf bQAcdaNnNW5JA== Original-Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4F1Nt13Z0Tz9rxY; Thu, 18 Mar 2021 11:43:37 +0100 (CET) In-reply-to: Autocrypt: addr=thievol@posteo.net; prefer-encrypt=mutual; keydata= mQGNBF8ylcIBDADG+hy+zR6L4/vbdDDZuSaMmSrU3A5QZJpeBCvxTr7MpzzruZbhLPW1K3R6N2MA edi8Y+C8o27FVRIjpdbaKMGu9je7JV/TbUQYo3SOwCK1vM4LUn4V6ZLzSYkuiEt4eyMoiDdyvN0p kcK6P9x9DCetcEVszXzQg+yzCVrQ2hXWDXWT4M18EC3wtO7RHPouMqGiwBFhBAYErCqFWFxQHkfb tG/4yGyJ58rglb65O3qijjMWvYwcWZun9/7qm8Z4/4mHopmo2zgU+OrptnLSZfkZGz3Y7Uf452xQ GVq0Fv75NPvQru7y+DYVhuVXXyAmGxt+vf4rIiixMBbhKEPjcxEPAa2LTzex2IsTZR+QVG9uDnqC WcgaOEQ58fzXNvNhtwwF/Rgio2XWAJVdmFWS59/k9W58CIUSNKBMZh2XeGdEmtHvDtCxW3z6FJha 36RzOM3fMNNiAGdFZJA84gcdloJR+sHCDTTPT3784fjr+V8An7sI581NGFzkRQqPvEQCZbUAEQEA AbQSdGhpZXZvbEBwb3N0ZW8ubmV0iQHUBBMBCgA+FiEEI9twfRN7r3nig/xwDsVtFB0W75MFAl8y lcICGwMFCQPCZwAFCwkIBwIGFQoJCAsCBBYCAwECHgECF4AACgkQDsVtFB0W75MB3QwAlTsVzFmr +S/tMKwwwOibjhNPi/OZiUC2AYfaqfVAiIHDT3RbzDe03sAJoomJkJnYVjGzQZwibCMO2+ITkMPV 2wvrd4CbgS1KCVbrltwcuK/nxPCBaHytOCZUIInnhJo5PE/h03K Received-SPF: pass client-ip=185.67.36.65; envelope-from=thievol@posteo.net; helo=mout01.posteo.de X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:266555 Archived-At: --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hello, just taking this thread on the fly. Normally one can pass a list of files with absolute path to dired to get a dired buffer with only the needed file names to possibly edit with wdired, however this is broken in Emacs since ages; Helm is fixing this by advice, see https://github.com/emacs-helm/helm/blob/master/helm-lib.el#L186. This allow selecting files from helm-find-files and switching to a dired buffer with only those files. HTH. Arthur Miller writes: > Stefan Monnier writes: > >> 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). = ] > > I skipped it I don't like to look at it in either CL nor Elisp; in genera= l I > don't like much of special characters in syntax. I think it is pitty it > is introduced in Elisp. But if you want to have it, it is just to add > it, it's not something essential to argue about; I am just giving reason > why I didn't used it from the beginning :-). > >>> +(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. > > Can you elaborate more on this please? Why is it wrong and why do you > want it to work with region? As I see it used in Dired, most of the time > beg and end points will be the same, which will effectively result in > lots of processing to change properties for one character. Usually, in > Dired buffer I would move to a line with name and change few characters > only. > > Current code changes properties for entire line, if it worked on region > it would change only properties for one char at time. I think it turns > into very complicated processing for just changing one char properties. > > I checked to do some editing and printed out region when normally > editing a file name: > (defun wdired--preprocess-line (beg end) > "Preprocess current line under point to make it writable. " > (let ((inhibit-read-only t)) > (message "REG: %s %s" beg end) > ( ... ) > > This is what I got for each char I typed in: > > REG: 8397 8397 > REG: 8398 8398 > REG: 8399 8399 > REG: 8400 8400 > REG: 8401 8401 > REG: 8402 8402 > REG: 8402 8403 > REG: 8401 8402 > REG: 8400 8401 > REG: 8399 8400 > REG: 8398 8399 > REG: 8397 8398 > > The way I did it, it works per line; it will process entire line at > time. I have also checked to change multiple file names with rectangle > commands and in it works well too. I guess due to how Emacs internally > anserts chars into buffer (sequentially). > >> 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. ] > > Thanks; I wasn't aware of that macro either. > >>> @@ -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-l= ink 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. > > There are much bigger monstrousites I have seen in Emacs lisp folder > than this :-). > > I agree with you that we should avoid large functions, I had that in > mind when I refactored out those two methods, but I didn't thought this > was so big (~60 lines without comments). I merged them into one to fit > everything relevant into one spot and one page on the screen. I can > change it back to three smaller defuns, but I will still though prefer > to have them following each other in the code rather than scattered > around in wdired.el for no reason as it is now. They are not refered > from any other code. > > Anyway, sharps and refactoring are non-important issue; I can change it > back, just please check more on that with region, before I do changes. =2D-=20 Thierry --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQHHBAEBCgAxFiEEI9twfRN7r3nig/xwDsVtFB0W75MFAmBTLtcTHHRoaWV2b2xA cG9zdGVvLm5ldAAKCRAOxW0UHRbvkyBiC/0S6b9nLeiHm3VisPCS3/GXAk/M/Irv abkfNluuJ5AL9t71FbfwCEApt8VB+ju7WhzgiN4TYeLOnEOxvvAKcHIlYHR1qaV7 nopAUNuRpZf1UCZUb+0w6fwZyrr5Fq5sUuPx7ehqb2fX1KjaTwtjw6MIe6MRm8M5 yZqGMm+dE18alQr9w7vA4+8UfK2k4E1e3IFiiVVx2dEed9wnP39k09R5U6/LShES nbJtLIqmEicPp40VeGZfjs4vorGGjtXETy68gwHGQ1AZHHWywRJ2ZDEW8pb5sMgf PcRVH1/V2IY7kGQu39ArXnNMYUYaviHYgUKtP62J1Q8MDYv93/aWQYkdVkM6bPmy S62I+gUUz2GmtmeT3uVDjFVw7I4kGh+V2FzFAoEBfkOpFYcnYW7AtsiYLCFg8/ij W0VQ+h1p0cnoDrTIro09OfJ6yjp0pjmlIW7DxFTPyFQQ1oQ74P0GhGjRQodopemz kLR7KrE3kpKvf9YQFAxp5jLtqO/Xvw3QO10= =J1+T -----END PGP SIGNATURE----- --=-=-=--