From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Arthur Miller Newsgroups: gmane.emacs.devel Subject: Re: Partial wdired (edit just filename at the point) Date: Thu, 18 Mar 2021 11:26:14 +0100 Message-ID: References: Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="35969"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Thu Mar 18 11:28:10 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 1lMpsv-0009E8-Cm for ged-emacs-devel@m.gmane-mx.org; Thu, 18 Mar 2021 11:28:09 +0100 Original-Received: from localhost ([::1]:44972 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lMpsu-000613-AH for ged-emacs-devel@m.gmane-mx.org; Thu, 18 Mar 2021 06:28:08 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:56258) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lMprB-0004J3-W4 for emacs-devel@gnu.org; Thu, 18 Mar 2021 06:26:22 -0400 Original-Received: from mail-am7eur06olkn2050.outbound.protection.outlook.com ([40.92.16.50]:7008 helo=EUR06-AM7-obe.outbound.protection.outlook.com) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lMpr8-00033K-PN for emacs-devel@gnu.org; Thu, 18 Mar 2021 06:26:21 -0400 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=TfZtdBH4UhAn/rdgC1lV+f44XlItHg2LJjHuAAa+dvfDcoGroZQkSOkla9OfWpFVUG0wWN/NOSYXLPlFwEarehToG9PtmASvGJX5bCfxWPFsC8HGcACsp++mXij8Ldb4JzAtyH4CXpjcTdbZnhos6WJ5e7jwR9nhp77HV+XUHIxDiH30RvpyUV2OwE/W7RmeFQus278c9b6jC5AWIW8LJLuvt9Tp3/nE0peT0F9Kxt4NzK/pisSS1DEYC4tnujo45dKf/uq2+OpEQcfbPKND4bcD83gXlQyrLJZV1nbP1wtdpbPG+53DyJ1iwqRFRwaaQdiOwKwyltjgdAF4uwalKw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=bpOCBmABD/QZ2uqhBW7NhyXUX1vsSkMXM9aLclLagKE=; b=ZbYMG0VMBZYo1FekxSUWfI4iBPvUwyfL8tVuknIfbkoPritK+lTpqSq0X8vDruyAiB2Mo1b7/cNXMSfCxyhJ8tUJ5+VmALVUMRuSoHyzY7JDfiv3UyvZVXsZ/Vg6nJUWvPLJU/M1lZH6oMZ6nCe/uQk8T/tbqkFbpkd0/+RHnPYyk683dKYzv23MVg8iTYbKmzpQDc3iTmxvdbgkR/r6r4twSc8kcmniKbQqHdzoo0bqlk6HG4tDv9CuNYfCmlhhbXhz5B0tcRL9KNm1sDK5JVoW61p5XZX+0jnDH3KQatx+dmBiv3R5Y3szfwVbSLod2pAz7C1i05I+cxlEyXhp7g== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=live.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=bpOCBmABD/QZ2uqhBW7NhyXUX1vsSkMXM9aLclLagKE=; b=DWiOhx1KjoZZCswVpmAWRCDBPM9siMXqznkwLKlKuPkWuAvKKswwijSXZssuxjfbRxgupZKqEnhC4/Rl9Qkrz1aGmDxn1isrAM48kWbOLT/1x9M1MmR/bDQ98347G6e0cIlA38RIDV1OQ4kpbKF4ePJQP5aHlm4Z+Lz9A1q2BnwLNqBgQd9mfMrlcM3OK6Uu3y1wqPxQlHdKrvWEQnujsSKsB6kdjFECAdATPwMjiOlLVaGzJGiG6VrxcqpvdPs4JQ2UBd2r5KmIDtH/SIOqk39TJ1zmg9KV0AXp32eotVQyMnMoaaIarvXDG/85hnxB5pU38qMjk55ttk3NIV7gig== Original-Received: from AM7EUR06FT050.eop-eur06.prod.protection.outlook.com (2a01:111:e400:fc36::48) by AM7EUR06HT137.eop-eur06.prod.protection.outlook.com (2a01:111:e400:fc36::305) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3955.18; Thu, 18 Mar 2021 10:26:16 +0000 Original-Received: from AM9PR09MB4977.eurprd09.prod.outlook.com (2a01:111:e400:fc36::53) by AM7EUR06FT050.mail.protection.outlook.com (2a01:111:e400:fc36::450) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3955.18 via Frontend Transport; Thu, 18 Mar 2021 10:26:16 +0000 X-IncomingTopHeaderMarker: OriginalChecksum:EA6F379D05E38D122D1F7940EFE595C95465B10DD6D1D5F2DB3A48777221A67E; UpperCasedChecksum:8E0BE1822DE26DD8536C4AA603B5CA57573BF93BFD7D0ADF5279DE368078AB2C; SizeAsReceived:7950; Count:46 Original-Received: from AM9PR09MB4977.eurprd09.prod.outlook.com ([fe80::2103:e705:bc0c:5a8b]) by AM9PR09MB4977.eurprd09.prod.outlook.com ([fe80::2103:e705:bc0c:5a8b%6]) with mapi id 15.20.3955.018; Thu, 18 Mar 2021 10:26:16 +0000 In-Reply-To: (Stefan Monnier's message of "Wed, 17 Mar 2021 22:10:25 -0400") X-TMN: [tI8GRVqAkFgNEq1YAxN/OapBKDY5dHHD] X-ClientProxiedBy: AM6P191CA0091.EURP191.PROD.OUTLOOK.COM (2603:10a6:209:8a::32) To AM9PR09MB4977.eurprd09.prod.outlook.com (2603:10a6:20b:304::20) X-Microsoft-Original-Message-ID: <87k0q4dagp.fsf@live.com> X-MS-Exchange-MessageSentRepresentingType: 1 Original-Received: from pascal.homepc (90.230.29.56) by AM6P191CA0091.EURP191.PROD.OUTLOOK.COM (2603:10a6:209:8a::32) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3955.18 via Frontend Transport; Thu, 18 Mar 2021 10:26:15 +0000 X-MS-PublicTrafficType: Email X-IncomingHeaderCount: 46 X-EOPAttributedMessage: 0 X-MS-Office365-Filtering-Correlation-Id: 5be5c681-0c55-49ad-9584-08d8e9f842e5 X-MS-TrafficTypeDiagnostic: AM7EUR06HT137: X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 33yXNiU0lYDYXs+NfIHbm76d4pp3vl/bree1y3GmYTTbdZDXwKs9QVGR0nkjHIfOFRwG4EIDPQb5mDErUDDNfx27BwpPBx9O9SRLL6qi0vJdu9LfxCk524lyXgf2Vm5pQQKyKS1cYLHb0WGYSH3QlgE7rMpPTad+9IqXlOZLgmSl69KjJ8TRW0v2BiWixWWPmPFSYvga5MO2Blvw89HEC2y/+9PPR83046L/ayL71IHTvKmtt3JKgqZXvUK+k2XfFOwtTNKDjKe5UYRQJKlfdO5qmb9UAZj9Uv4DRDBMVxSue1JYsTdPxPTyOggK9oi7h8PAo8XvyL+IkC/qsMCwJPOleGRqKIUPPmtdhvRofvQketgM0OMSM+XecJPYxOIEVe5RYIadN/8gsWJN1jh36g== X-MS-Exchange-AntiSpam-MessageData: QHPYbjatLSnAIByp9o997DY2MvYT/MwWPjZUDFw9fRda/mm/Zpa0X6VpqIBKgYXaKPYiQ9+uJvr8S9M93r8qIHw3LaLabLe3U66dxmGQjnFiMnYqi+/UAJ42UDadKjyA5Xg1biKGYvxoaHq2rDCqJQ== X-OriginatorOrg: live.com X-MS-Exchange-CrossTenant-Network-Message-Id: 5be5c681-0c55-49ad-9584-08d8e9f842e5 X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Mar 2021 10:26:16.0630 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-AuthSource: AM7EUR06FT050.eop-eur06.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: Internet X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM7EUR06HT137 Received-SPF: pass client-ip=40.92.16.50; envelope-from=arthur.miller@live.com; helo=EUR06-AM7-obe.outbound.protection.outlook.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 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, FREEMAIL_FROM=0.001, MSGID_FROM_MTA_HEADER=0.001, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_PASS=-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:266553 Archived-At: 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 general 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-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. 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.