From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Tino Calancha Newsgroups: gmane.emacs.devel Subject: Re: [Emacs-diffs] master 75d9a55: Fix bug 32543 Date: Wed, 19 Sep 2018 02:37:12 +0900 Message-ID: <87bm8u906v.fsf@calancha-pc.dy.bbexcite.jp> References: <20180918123203.24597.514@vcs0.savannah.gnu.org> <20180918123205.8BE9B204DF@vcs0.savannah.gnu.org> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: blaine.gmane.org 1537292167 32090 195.159.176.226 (18 Sep 2018 17:36:07 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 18 Sep 2018 17:36:07 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) Cc: tino.calancha@gmail.com, emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue Sep 18 19:36:03 2018 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1g2Juq-00085v-Fi for ged-emacs-devel@m.gmane.org; Tue, 18 Sep 2018 19:36:00 +0200 Original-Received: from localhost ([::1]:41436 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1g2Jww-00026x-T2 for ged-emacs-devel@m.gmane.org; Tue, 18 Sep 2018 13:38:10 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:50763) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1g2JwB-00020N-2N for emacs-devel@gnu.org; Tue, 18 Sep 2018 13:37:24 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1g2Jw7-0002be-Sg for emacs-devel@gnu.org; Tue, 18 Sep 2018 13:37:23 -0400 Original-Received: from mail-wm1-x32f.google.com ([2a00:1450:4864:20::32f]:37010) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1g2Jw7-0002aj-ID for emacs-devel@gnu.org; Tue, 18 Sep 2018 13:37:19 -0400 Original-Received: by mail-wm1-x32f.google.com with SMTP id n11-v6so3799695wmc.2 for ; Tue, 18 Sep 2018 10:37:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=5pW2Yp1l+Hnku75KhNl+IUtDpyPvt2H/8Q8jvEOmxfA=; b=lCvFN68SqlJv4DGLrgSZH/6jhv5FU/lr8fv36mmoHCIHMudwzOAoUUTrr8FFMi491P QwleQVheY81dvwfafNAHhTSufQ7ASo7gUAoVCWxC8TBGYHhISnfOwfIx/dmJQCR6fabn zb6KbzQECs7PQLHYoiJmfYW1CxyPLkOlD4I3cVdEzrK3WVEoPKVklYyaBM+/jiWWc5HL Lay013LPp5wTI7nRVrEk/8w6nW5PCoNn38l86wQbuLDdndIMo7nHnzeGqDNqPflcsdVO J4xboN+rgYWHnR0QVXYCGXa4/8DFMYiNMq8L8dhOZ+5KXxXhC5AUqIQIGdpNLso+0Cz6 ZmIA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=5pW2Yp1l+Hnku75KhNl+IUtDpyPvt2H/8Q8jvEOmxfA=; b=CRxcmnJigDQi7N4Qz4fB0B46HhNlaK6J8VXM46YD19Ylp2s4KDcdqoQN1owpd9pBHW y9SmWjvko/thQtFtDOLC4Raujes39jZy5CSqjc1bXtYx9UzhReXPWyntN2iPzK+GHz77 InwnGPgp73oAdHg6T6p6vB/73d94VCgJuZ9nEmNiT/7FHymzVjA55Tz546Q5DRwL275e g4m46BF7Ik+PBwYRm68dhKXdQqOY67ZR+N9t0P7AKe7jwtLy48W+QAzIAtXyexD9Dsgg YjzhklnnN93jVpReTaP/+5/RyiWe4GrR+Vt5Kt9XZCtL/IpUDp8+wT6SkXhsoFzueHlg Z3cA== X-Gm-Message-State: APzg51CzpbKYX54/SHoBIb23mgcS2X1tFALyG934s/39N6ghne6DKzU0 554TsSHG1vQ3nlNJhzdDkqI= X-Google-Smtp-Source: ANB0VdbCqRRn6GNaKPh2mWYCTB69c4byNZueYMBNUjoeNRpsItnfhiBcEBvRzJRzXZbRW9CGuLsbBg== X-Received: by 2002:a1c:7e92:: with SMTP id z140-v6mr16400395wmc.48.1537292238288; Tue, 18 Sep 2018 10:37:18 -0700 (PDT) Original-Received: from calancha-pc.dy.bbexcite.jp (15.red-83-50-151.dynamicip.rima-tde.net. [83.50.151.15]) by smtp.gmail.com with ESMTPSA id s24-v6sm1848698wmc.7.2018.09.18.10.37.16 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 18 Sep 2018 10:37:17 -0700 (PDT) In-Reply-To: (Stefan Monnier's message of "Tue, 18 Sep 2018 08:56:52 -0400") X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2a00:1450:4864:20::32f X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 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.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:229949 Archived-At: Stefan Monnier writes: >> +(defun occur--parse-occur-buffer() >> + "Retrieve a list of the form (BEG END ORIG-LINE BUFFER). >> +BEG and END define the region. >> +ORIG-LINE and BUFFER are the line and the buffer from which >> +the user called `occur'." >> + (save-excursion >> + (goto-char (point-min)) >> + (let ((buffer (get-text-property (point-at-bol) 'occur-title)) >> + (beg-pos (get-text-property (point-at-bol) 'region-start)) >> + (end-pos (get-text-property (point-at-bol) 'region-end)) >> + (orig-line (get-text-property (point-at-bol) 'current-line)) >> + beg-line end-line) >> + (list beg-pos end-pos orig-line buffer)))) > > I'm curious here: > - Why use (point-at-bol) since we just moved to (point-min) and hence we > know that (point-at-bol) == (point-min) == (point)? In a previous patch I saved the info in one particular position other that (point-min); finally I saved the info in the entire line as it is done with the buffer. > - why store this info in text-properties rather than in buffer-local variables? I realized the buffer was already saved there with property `occur-title'; I didn't wanted to scatter around related info. >> + (let* ((region (occur--parse-occur-buffer)) >> + (region-start (nth 0 region)) >> + (region-end (nth 1 region)) >> + (orig-line (nth 2 region)) >> + (buffer (nth 3 region)) >> + (regexp (car occur-revert-arguments))) > > Here you could have used pcase-let: > > (pcase-let ((`(,region-start ,region-end ,orig-line ,buffer) > (occur--parse-occur-buffer)) > (regexp (car occur-revert-arguments))) Nicer. Thank you! >> + (with-current-buffer buffer >> + (when (wholenump orig-line) >> + (goto-char 1) > > I'd recommend `point-min` instead of 1 here. OK. I always remember the discussion at https://lists.gnu.org/archive/html/emacs-devel/2009-08/msg00520.html but I forgot which was the encouraged practice there: in these cases I follow the 50% rule to reject 0% success ratio (I know, it brings 50% failure ratio: life is full of injustice). >> + (forward-line (1- orig-line))) >> + (save-excursion >> + (if region >> + (occur regexp nil (list (cons region-start region-end))) >> + (apply 'occur-1 (append occur-revert-arguments (list (buffer-name)))))))))) > > `region` is the list which holds (among other things) `buffer`, so if we > successfully entered (with-current-buffer buffer ...) I can't see how > `region` could be nil. Good catch! It looks like my typo. Probably I wanted here to check if we have called `occur' before with a non-nil region, i.e., this check: + (if (or region-start region-end) I can push following amend commit: --8<-----------------------------cut here---------------start------------->8--- --- a/lisp/replace.el +++ b/lisp/replace.el @@ -1213,29 +1213,25 @@ occur--parse-occur-buffer the user called `occur'." (save-excursion (goto-char (point-min)) - (let ((buffer (get-text-property (point-at-bol) 'occur-title)) - (beg-pos (get-text-property (point-at-bol) 'region-start)) - (end-pos (get-text-property (point-at-bol) 'region-end)) - (orig-line (get-text-property (point-at-bol) 'current-line)) - beg-line end-line) + (let ((buffer (get-text-property (point) 'occur-title)) + (beg-pos (get-text-property (point) 'region-start)) + (end-pos (get-text-property (point) 'region-end)) + (orig-line (get-text-property (point) 'current-line))) (list beg-pos end-pos orig-line buffer)))) (defun occur-revert-function (_ignore1 _ignore2) "Handle `revert-buffer' for Occur mode buffers." (if (cdr (nth 2 occur-revert-arguments)) ; multi-occur (apply 'occur-1 (append occur-revert-arguments (list (buffer-name)))) - (let* ((region (occur--parse-occur-buffer)) - (region-start (nth 0 region)) - (region-end (nth 1 region)) - (orig-line (nth 2 region)) - (buffer (nth 3 region)) - (regexp (car occur-revert-arguments))) + (pcase-let ((`(,region-start ,region-end ,orig-line ,buffer) + (occur--parse-occur-buffer)) + (regexp (car occur-revert-arguments))) (with-current-buffer buffer (when (wholenump orig-line) - (goto-char 1) + (goto-char (point-min)) (forward-line (1- orig-line))) (save-excursion - (if region + (if (or region-start region-end) (occur regexp nil (list (cons region-start region-end))) (apply 'occur-1 (append occur-revert-arguments (list (buffer-name)))))))))) --8<-----------------------------cut here---------------end--------------->8---