GNU bug report logs -
#24725
25.1.50; vc-region-history may exceed max line number of file in repository
Previous Next
Reported by: Tino Calancha <tino.calancha <at> gmail.com>
Date: Tue, 18 Oct 2016 14:28:01 UTC
Severity: normal
Tags: patch
Found in version 25.1.50
Done: Tino Calancha <tino.calancha <at> gmail.com>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 24725 in the body.
You can then email your comments to 24725 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#24725; Package
emacs.
(Tue, 18 Oct 2016 14:28:01 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Tino Calancha <tino.calancha <at> gmail.com>:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org.
(Tue, 18 Oct 2016 14:28:01 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
emacs -Q lisp/vc/vc.el
C-x h
M-x vc-region-history RET
fatal: file vc.el has only 2921 lines
;; vc.el has 2922 lines but Git ignores the last empty line.
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From 12a6fc588fe60ebf98443a0fd068fe77d63bd17e Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha <at> gmail.com>
Date: Tue, 18 Oct 2016 23:20:07 +0900
Subject: [PATCH] vc-region-history: Do not exceed the file maximum line number
The last empty line in a file is not part of the Git repository.
* lisp/vc/vc.el (vc-region-history): Exclude the last empty line
from the region (Bug#24724).
---
lisp/vc/vc-git.el | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 9eac5b2..762f6af 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1016,6 +1016,8 @@ vc-git-region-history
;; FIXME: Maybe this should be done in vc.el (i.e. for all backends), but
;; since Git is the only backend to support this operation so far, it's hard
;; to tell.
+ (when (> lto (1- (line-number-at-pos (point-max))))
+ (setq lto (1- (line-number-at-pos (point-max)))))
(with-temp-buffer
(vc-call-backend 'git 'diff file "HEAD" nil (current-buffer))
(goto-char (point-min))
--
2.9.3
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
In GNU Emacs 25.1.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.1)
of 2016-10-13 built on calancha-pc
Repository revision: baa8ba4ed471d7fe4bb07c80a9dd16c4712525b4
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#24725; Package
emacs.
(Wed, 19 Oct 2016 23:23:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 24725 <at> debbugs.gnu.org (full text, mbox):
Hi!
On 18.10.2016 17:27, Tino Calancha wrote:
>
> emacs -Q lisp/vc/vc.el
> C-x h
> M-x vc-region-history RET
> fatal: file vc.el has only 2921 lines
> ;; vc.el has 2922 lines but Git ignores the last empty line.
With your patch, what will happen if the file does not end with a
newline (which can be some people's or tools' preference)?
Currently, in that case the above scenario works fine. Maybe we need a
different check, rather than substracting.
Or try (line-number-at-pos (1- (point-max))) instead of (1-
(line-number-at-pos (point-max))).
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#24725; Package
emacs.
(Thu, 20 Oct 2016 04:23:01 GMT)
Full text and
rfc822 format available.
Message #11 received at 24725 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> Hi!
>
> On 18.10.2016 17:27, Tino Calancha wrote:
>>
>> emacs -Q lisp/vc/vc.el
>> C-x h
>> M-x vc-region-history RET
>> fatal: file vc.el has only 2921 lines
>> ;; vc.el has 2922 lines but Git ignores the last empty line.
>
> With your patch, what will happen if the file does not end with a
> newline (which can be some people's or tools' preference)?
In that case my patch is wrong: it excludes the last line from the region.
> Currently, in that case the above scenario works fine. Maybe we need a
> different check, rather than substracting.
>
> Or try (line-number-at-pos (1- (point-max))) instead of (1-
> (line-number-at-pos (point-max))).
Thank you. Your suggestion handle both cases correctly.
Following is the new patch:
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From c9ce3a54f572549ea2027108694eb556b5cbe8f3 Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha <at> gmail.com>
Date: Thu, 20 Oct 2016 12:22:51 +0900
Subject: [PATCH] vc-region-history: Do not exceed the file maximum line number
The last empty line in a file is not part of the Git repository.
* lisp/vc/vc.el (vc-region-history): Exclude the last empty line
from the region (Bug#24725).
---
lisp/vc/vc-git.el | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 9eac5b2..c6a9064 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1016,6 +1016,8 @@ vc-git-region-history
;; FIXME: Maybe this should be done in vc.el (i.e. for all backends), but
;; since Git is the only backend to support this operation so far, it's hard
;; to tell.
+ (when (> lto (line-number-at-pos (1- (point-max))))
+ (setq lto (line-number-at-pos (1- (point-max)))))
(with-temp-buffer
(vc-call-backend 'git 'diff file "HEAD" nil (current-buffer))
(goto-char (point-min))
--
2.9.3
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
In GNU Emacs 25.1.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.1)
of 2016-10-20
Repository revision: 8988327d548db7b69f30ea15496ccb0726fa4502
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#24725; Package
emacs.
(Thu, 20 Oct 2016 07:28:02 GMT)
Full text and
rfc822 format available.
Message #14 received at 24725 <at> debbugs.gnu.org (full text, mbox):
On Okt 20 2016, Tino Calancha <tino.calancha <at> gmail.com> wrote:
> diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
> index 9eac5b2..c6a9064 100644
> --- a/lisp/vc/vc-git.el
> +++ b/lisp/vc/vc-git.el
> @@ -1016,6 +1016,8 @@ vc-git-region-history
> ;; FIXME: Maybe this should be done in vc.el (i.e. for all backends), but
> ;; since Git is the only backend to support this operation so far, it's hard
> ;; to tell.
> + (when (> lto (line-number-at-pos (1- (point-max))))
> + (setq lto (line-number-at-pos (1- (point-max)))))
Shouldn't that be fixed in the caller, and lto always be decremented if
the regions ends at bol?
Andreas.
--
Andreas Schwab, SUSE Labs, schwab <at> suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#24725; Package
emacs.
(Thu, 20 Oct 2016 08:10:01 GMT)
Full text and
rfc822 format available.
Message #17 received at 24725 <at> debbugs.gnu.org (full text, mbox):
Andreas Schwab <schwab <at> suse.de> writes:
> On Okt 20 2016, Tino Calancha <tino.calancha <at> gmail.com> wrote:
>
>> diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
>> index 9eac5b2..c6a9064 100644
>> --- a/lisp/vc/vc-git.el
>> +++ b/lisp/vc/vc-git.el
>> @@ -1016,6 +1016,8 @@ vc-git-region-history
>> ;; FIXME: Maybe this should be done in vc.el (i.e. for all backends), but
>> ;; since Git is the only backend to support this operation so far, it's hard
>> ;; to tell.
>> + (when (> lto (line-number-at-pos (1- (point-max))))
>> + (setq lto (line-number-at-pos (1- (point-max)))))
>
> Shouldn't that be fixed in the caller, and lto always be decremented if
> the regions ends at bol?
I think you are right. Otherwise we are including a line out of the
region, i.e., in interactive calls we also search in the history for
a line out of the highlighted region.
How about following new patch?
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From b05b6d12171ea270f6b392c8af7f4415414960e5 Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha <at> gmail.com>
Date: Thu, 20 Oct 2016 17:00:04 +0900
Subject: [PATCH] vc-region-history: Search just on lines within the region
* lisp/vc/vc.el (vc-region-history): If region ends in the beginning
of a line, then exclude that line from the search (Bug#24725).
---
lisp/vc/vc.el | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index af875e8..0d8e308 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -2393,7 +2393,11 @@ vc-region-history
"Show the history of the region FROM..TO."
(interactive "r")
(let* ((lfrom (line-number-at-pos from))
- (lto (line-number-at-pos to))
+ (lto (save-excursion
+ (goto-char to)
+ (if (bolp)
+ (1- (line-number-at-pos to))
+ (line-number-at-pos to))))
(file buffer-file-name)
(backend (vc-backend file))
(buf (get-buffer-create "*VC-history*")))
--
2.9.3
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
In GNU Emacs 25.1.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.1)
of 2016-10-20
Repository revision: 8988327d548db7b69f30ea15496ccb0726fa4502
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#24725; Package
emacs.
(Thu, 20 Oct 2016 10:22:01 GMT)
Full text and
rfc822 format available.
Message #20 received at 24725 <at> debbugs.gnu.org (full text, mbox):
On 20.10.2016 11:09, Tino Calancha wrote:
>> Shouldn't that be fixed in the caller, and lto always be decremented if
>> the regions ends at bol?
> I think you are right. Otherwise we are including a line out of the
> region, i.e., in interactive calls we also search in the history for
> a line out of the highlighted region.
> How about following new patch?
LGTM, please install.
Although I was thinking of simplifying it to just:
(line-number-at-pos (1- to))
That can only bite us if TO is at bob, which would be very unusual.
Reply sent
to
Tino Calancha <tino.calancha <at> gmail.com>:
You have taken responsibility.
(Thu, 20 Oct 2016 10:46:01 GMT)
Full text and
rfc822 format available.
Notification sent
to
Tino Calancha <tino.calancha <at> gmail.com>:
bug acknowledged by developer.
(Thu, 20 Oct 2016 10:46:01 GMT)
Full text and
rfc822 format available.
Message #25 received at 24725-done <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 20.10.2016 11:09, Tino Calancha wrote:
>
>>> Shouldn't that be fixed in the caller, and lto always be decremented if
>>> the regions ends at bol?
>> I think you are right. Otherwise we are including a line out of the
>> region, i.e., in interactive calls we also search in the history for
>> a line out of the highlighted region.
>> How about following new patch?
>
> LGTM, please install.
>
> Although I was thinking of simplifying it to just:
>
> (line-number-at-pos (1- to))
>
> That can only bite us if TO is at bob, which would be very unusual.
Good. It sounds more simple.
Pushed that simple fix to emacs-25 branch as commit 3877c911
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Thu, 17 Nov 2016 12:24:03 GMT)
Full text and
rfc822 format available.
This bug report was last modified 8 years and 211 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.