Re: bug#27644: 26.0.50; [PATCH] Add binding for vc-region-history

Re: bug#27644: 26.0.50; [PATCH] Add binding for vc-region-history

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 1345 bytes --]

Hello all,

In https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27644, I propose a patch
to bind the vc-region-history command to C-x v H.

* Why vc-region-history?
- I find that command very useful; especially to dig though who
specifically touched a set of lines in emacs code and when. I used it very
recently[1] to associate a strange behavior I started seeing in ggtags.el
package with an upstream change in emacs.

* Why bind to C-x v H
- That binding is an easy mnemonic; the "H" in C-x v H for
vc-region-(H)istory.
- That binding is available

So the questions to the list:
- How frequently do you guys use vc-region-history?
- If you bind it, what do you bind it to?
- Is there a reason why this patch should not be applied?

Thanks all.

Kaushal

[1]: https://github.com/leoliu/ggtags/issues/157#issuecomment-330270080



On Thu, Sep 21, 2017 at 11:29 AM Dmitry Gutov <dgutov@yandex.ru> wrote:

> On 9/21/17 5:41 PM, Eli Zaretskii wrote:
>
> >> I don't have a strong opinion.
> >
> > About binding this to a key in general, or about the particular key
> > proposed by Kaushal?
>
> Either. It's not a very technical decision, and I very rarely use this
> command myself.
>
> > If the former, perhaps we should ask how many
> > others bind this to a key, or how frequently they use this command.
>
> Sounds good to me.
>

-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 2236 bytes --]

Re: bug#27644: 26.0.50; [PATCH] Add binding for vc-region-history

[Stefan Monnier]

> - How frequently do you guys use vc-region-history?

All the time.

> - If you bind it, what do you bind it to?

C-x v h

I've never found the default binding of C-x v h to be of any use.


        Stefan

Re: bug#27644: 26.0.50; [PATCH] Add binding for vc-region-history

[Dmitry Gutov]

On 9/22/17 6:15 AM, Stefan Monnier wrote:
>> - How frequently do you guys use vc-region-history?
> 
> All the time.
> 
>> - If you bind it, what do you bind it to?
> 
> C-x v h
> 
> I've never found the default binding of C-x v h to be of any use.

If we do end up adding a binding, this makes a lot of sense.

vc-insert-headers only seems useful with older VCS-es, and even there, 
not particularly essential.

Not sure we can do this change this close to Emacs 26 release, though. 
That's up to the current maintainers.

Re: bug#27644: 26.0.50; [PATCH] Add binding for vc-region-history

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 330 bytes --]

On Thu, Sep 21, 2017 at 11:20 PM Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> > - If you bind it, what do you bind it to?
>
> C-x v h
>
> I've never found the default binding of C-x v h to be of any use.
>

Well, I couldn't be happier. I also never found the C-x v h binding useful.
Thanks for the input.
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 733 bytes --]

Re: bug#27644: 26.0.50; [PATCH] Add binding for vc-region-history

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 885 bytes --]

On Fri, Sep 22, 2017 at 9:51 AM Dmitry Gutov <dgutov@yandex.ru> wrote:

> >> - If you bind it, what do you bind it to?
> >
> > C-x v h
> >
> > I've never found the default binding of C-x v h to be of any use.
>
> If we do end up adding a binding, this makes a lot of sense.
>

+1


> Not sure we can do this change this close to Emacs 26 release, though.
> That's up to the current maintainers.
>

It a matter of changing just the binding. It's not as if a new
functionality or change in older functionality needs to be thoroughly
tested.

Also, I made that patch proposal back in July[1]. And seeing that we still
need to get the first 26.1 pretest out, I thought it might not be too late
for this just-binding change. I hope the maintainers can ship it with emacs
26.1. If not that, at least in the master.

[1]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27644
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1570 bytes --]

Re: bug#27644: 26.0.50; [PATCH] Add binding for vc-region-history

[Clément Pit-Claudel]

On 2017-09-22 05:15, Stefan Monnier wrote:
>> - How frequently do you guys use vc-region-history?
> All the time.
> 
>> - If you bind it, what do you bind it to?
> C-x v h

+1

Re: bug#27644: 26.0.50; [PATCH] Add binding for vc-region-history

[Jostein Kjønigsen]

[-- Attachment #1: Type: text/plain, Size: 429 bytes --]


On Sat, Sep 23, 2017, at 10:06 AM, Clément Pit-Claudel wrote:
> On 2017-09-22 05:15, Stefan Monnier wrote:
>>> - How frequently do you guys use vc-region-history?
>> All the time.
>> 
>>> - If you bind it, what do you bind it to?
>> C-x v h
> 
> +1

Just another voice seconding this sentiment.

--
Regards
Jostein Kjønigsen

jostein@kjonigsen.net 🍵 jostein@gmail.com
https://jostein.kjonigsen.net




[-- Attachment #2: Type: text/html, Size: 1295 bytes --]

Re: bug#27644: 26.0.50; [PATCH] Add binding for vc-region-history

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 367 bytes --]

On Thu, Sep 28, 2017, 7:11 AM Jostein Kjønigsen <
jostein@secure.kjonigsen.net> wrote:

>
> Just another voice seconding this sentiment.
>

Thank you.

Thank you everyone. So far the concensus is to set the binding for
vc-region-history to C-x v h.

@Eli: Are there any objections? If not, I'll update the patch for
committing.

> --

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 970 bytes --]

Re: bug#27644: 26.0.50; [PATCH] Add binding for vc-region-history

[John Wiegley]

>>>>> "KM" == Kaushal Modi <kaushal.modi@gmail.com> writes:

KM> Thank you everyone. So far the concensus is to set the binding for
KM> vc-region-history to C-x v h.

KM> @Eli: Are there any objections? If not, I'll update the patch for
KM> committing.

Isn't C-x v h already `vc-insert-headers'?

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: bug#27644: 26.0.50; [PATCH] Add binding for vc-region-history

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 1386 bytes --]

On Thu, Sep 28, 2017 at 12:59 PM John Wiegley <jwiegley@gmail.com> wrote:

> Isn't C-x v h already `vc-insert-headers'?
>

Hi John,

As mentioned earlier in the thread, Stefan suggested this binding as he
never found the default binding useful[1], Dmitry corroborated that[2], I
have never found the default useful. Later Clement and Jostein also chimed
in a +1[3, 4].

So far there has been no opposition to binding C-x v h with
vc-region-history.

I am basically proposing to bind the super-useful vc-region-history to
"something" in "C-x v" map. I offered C-x v H. But based on the above
discussion C-x v h makes more sense.

I would be great to get a clear "yes" or "no" as this issue seems to have
lingered too long.

I'd put this very concisely as:

(let ((res (binding--vc-region-history--resolution)))
  (cond ((string= "yes, C-x v h" res)
         "best option")
        ((string= "yes, C-x v H" res)
         "good option")
        ((string= "no, don't bind vc-region-history" res)
         "bad option, but I'll accept")
        (t
         (error "invalid"))))



[1]: http://lists.gnu.org/archive/html/emacs-devel/2017-09/msg00772.html
[2]: http://lists.gnu.org/archive/html/emacs-devel/2017-09/msg00786.html
[3]: http://lists.gnu.org/archive/html/emacs-devel/2017-09/msg00823.html
[4]: http://lists.gnu.org/archive/html/emacs-devel/2017-09/msg00950.html
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 2575 bytes --]

Re: bug#27644: 26.0.50; [PATCH] Add binding for vc-region-history

[John Wiegley]

>>>>> Kaushal Modi <kaushal.modi@gmail.com> writes:

> So far there has been no opposition to binding C-x v h with
> vc-region-history.

> I am basically proposing to bind the super-useful vc-region-history to
> "something" in "C-x v" map. I offered C-x v H. But based on the above
> discussion C-x v h makes more sense.

> I would be great to get a clear "yes" or "no" as this issue seems to have
> lingered too long.

> I'd put this very concisely as:

> (let ((res (binding--vc-region-history--resolution)))
>   (cond ((string= "yes, C-x v h" res)
>          "best option")
>         ((string= "yes, C-x v H" res)
>          "good option")
>         ((string= "no, don't bind vc-region-history" res)
>          "bad option, but I'll accept")
>         (t
>          (error "invalid"))))

Hah, I love it. :)

I'm all for making it C-x v h, and having C-x v H insert headers. The former
is by far the more common operation, and in this case I'm willing to break
with tradition for better convenience. Please give this a good notice in the
NEWS file, so people notice.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: bug#27644: 26.0.50; [PATCH] Add binding for vc-region-history

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 547 bytes --]

On Thu, Sep 28, 2017 at 1:24 PM John Wiegley <jwiegley@gmail.com> wrote:

> Hah, I love it. :)
>
> I'm all for making it C-x v h, and having C-x v H insert headers. The
> former
> is by far the more common operation, and in this case I'm willing to break
> with tradition for better convenience. Please give this a good notice in
> the
> NEWS file, so people notice.
>

Thank you. I have emailed the patch (re-based to emacs-26 HEAD) on the
debbugs # 27644 thread[1].

[1]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27644#26
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1031 bytes --]

Re: bug#27644: 26.0.50; [PATCH] Add binding for vc-region-history

[Stefan Monnier]

> Isn't C-x v h already `vc-insert-headers'?

Indeed.  Have you ever used it?  Do you know of someone who ever used it?


        Stefan

Re: bug#27644: 26.0.50; [PATCH] Add binding for vc-region-history

[Stefan Monnier]

> I'm all for making it C-x v h,

Great.

> and having C-x v H insert headers.

Is it even worth it?


        Stefan

Re: bug#27644: 26.0.50; [PATCH] Add binding for vc-region-history

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > >>> - If you bind it, what do you bind it to?
  > >> C-x v h

It seems sensible to me.

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

Re: bug#27644: 26.0.50; [PATCH] Add binding for vc-region-history

[Robert Pluim]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Isn't C-x v h already `vc-insert-headers'?
>
> Indeed.  Have you ever used it?  Do you know of someone who ever used it?

Only the rcs, cvs, sccs and svn backends support it (and svn only for
backwards compatibility with CVS), and inserting vc headers has been
recommended against for at least 15 years if not longer. I say replace
the binding.

Robert

Re: bug#27644: 26.0.50; [PATCH] Add binding for vc-region-history

[Paul Eggert]

On 09/29/2017 02:37 AM, Robert Pluim wrote:
> Only the rcs, cvs, sccs and svn backends support it (and svn only for
> backwards compatibility with CVS), and inserting vc headers has been
> recommended against for at least 15 years if not longer.

I also suggest changing C-x v h to be something more useful. Although 
I'm a co-maintainer for RCS and still use RCS for some things, I've 
never used C-x v h with its old meaning.

Re: bug#27644: 26.0.50; [PATCH] Add binding for vc-region-history

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 818 bytes --]

On Fri, Sep 29, 2017 at 3:51 PM Paul Eggert <eggert@cs.ucla.edu> wrote:

> On 09/29/2017 02:37 AM, Robert Pluim wrote:
> > Only the rcs, cvs, sccs and svn backends support it (and svn only for
> > backwards compatibility with CVS), and inserting vc headers has been
> > recommended against for at least 15 years if not longer.
>
> I also suggest changing C-x v h to be something more useful. Although
> I'm a co-maintainer for RCS and still use RCS for some things, I've
> never used C-x v h with its old meaning.
>

That's good to hear! That adds to the consensus that vc-insert-headings is
not useful.

I have provided a patch that binds C-x v h to vc-region-history history in
the debbugs thread[1] .. just need someone to commit that.

[1]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27644#35
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1352 bytes --]