* bug#19548: VC changes under-documented, needlessly incompatible
@ 2015-01-09 17:41 Glenn Morris
2016-05-08 23:42 ` Dmitry Gutov
0 siblings, 1 reply; 11+ messages in thread
From: Glenn Morris @ 2015-01-09 17:41 UTC (permalink / raw)
To: 19548
Package: emacs
Version: 25.0.50
There have been extensive changes to VC in master, but there is zero
mention of any of it in NEWS. Normally one lists every changed
user-option and non-internal function.
Also, the changes are more incompatible than they need to be.
For example, in Emacs 24.4 there is a user option vc-cvs-stay-local.
In master, it was deleted in favour of vc-stay-local.
Use of define-obsolete-variable-alias would ease the transition.
Probably there are other such cases, as well as cases where setting
advertised-calling-convention rather than simply deleting function
arguments would help.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#19548: VC changes under-documented, needlessly incompatible
2015-01-09 17:41 bug#19548: VC changes under-documented, needlessly incompatible Glenn Morris
@ 2016-05-08 23:42 ` Dmitry Gutov
2016-05-13 21:05 ` Glenn Morris
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Dmitry Gutov @ 2016-05-08 23:42 UTC (permalink / raw)
To: Glenn Morris, 19548; +Cc: Eric S. Raymond
On 01/09/2015 07:41 PM, Glenn Morris wrote:
Sorry about saying it this late, but:
> Also, the changes are more incompatible than they need to be.
> For example, in Emacs 24.4 there is a user option vc-cvs-stay-local.
> In master, it was deleted in favour of vc-stay-local.
Looking at the commit that made this (185320a5), I see no reason for
there to be vc-stay-local. Quite the opposite, the commit message says:
"The CVS back end retaiin this machibery and the vc-stay-local
configuration variable now only affects it."
Why don't we remove vc-stay-local instead, and keep the appropriately
named vc-cvs-stay-local?
Do we expect vc-stay-local to have been a lot more popular to customize?
Then indeed it can become an alias.
> Probably there are other such cases, as well as cases where setting
> advertised-calling-convention rather than simply deleting function
> arguments would help.
If you could point out the specific ones, that would be helpful.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#19548: VC changes under-documented, needlessly incompatible
2016-05-08 23:42 ` Dmitry Gutov
@ 2016-05-13 21:05 ` Glenn Morris
2016-05-14 7:41 ` Eli Zaretskii
2016-05-15 23:37 ` Dmitry Gutov
2016-05-23 17:36 ` Eli Zaretskii
2 siblings, 1 reply; 11+ messages in thread
From: Glenn Morris @ 2016-05-13 21:05 UTC (permalink / raw)
To: 19548
Dmitry Gutov wrote:
>> Probably there are other such cases, as well as cases where setting
>> advertised-calling-convention rather than simply deleting function
>> arguments would help.
>
> If you could point out the specific ones, that would be helpful.
I can't be bothered to check. If no-one else can either, then obv.
there's no point keeping this report open - feel free to close it.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#19548: VC changes under-documented, needlessly incompatible
2016-05-13 21:05 ` Glenn Morris
@ 2016-05-14 7:41 ` Eli Zaretskii
0 siblings, 0 replies; 11+ messages in thread
From: Eli Zaretskii @ 2016-05-14 7:41 UTC (permalink / raw)
To: Glenn Morris; +Cc: 19548
> From: Glenn Morris <rgm@gnu.org>
> Date: Fri, 13 May 2016 17:05:26 -0400
>
> Dmitry Gutov wrote:
>
> >> Probably there are other such cases, as well as cases where setting
> >> advertised-calling-convention rather than simply deleting function
> >> arguments would help.
> >
> > If you could point out the specific ones, that would be helpful.
>
> I can't be bothered to check. If no-one else can either, then obv.
> there's no point keeping this report open - feel free to close it.
Can you list the issues that prompted you to give the bug report its
subject line? I'm sure you bumped into more than one of them. In
addition to the few mentioned explicitly in the OP, are there any
others?
Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#19548: VC changes under-documented, needlessly incompatible
2016-05-08 23:42 ` Dmitry Gutov
2016-05-13 21:05 ` Glenn Morris
@ 2016-05-15 23:37 ` Dmitry Gutov
2016-05-23 17:37 ` Eli Zaretskii
2016-05-23 17:36 ` Eli Zaretskii
2 siblings, 1 reply; 11+ messages in thread
From: Dmitry Gutov @ 2016-05-15 23:37 UTC (permalink / raw)
To: Glenn Morris, 19548; +Cc: Eric S. Raymond
On 05/09/2016 02:42 AM, Dmitry Gutov wrote:
> Why don't we remove vc-stay-local instead, and keep the appropriately
> named vc-cvs-stay-local?
>
> Do we expect vc-stay-local to have been a lot more popular to customize?
> Then indeed it can become an alias.
On the other hand, vc-cvs-stay-local-p contains this bit of code:
(let* ((sym (vc-make-backend-sym 'CVS 'stay-local))
(stay-local (if (boundp sym) (symbol-value sym) vc-stay-local)))
which seems like it will ensure that if the user has customized both
vc-stay-local and vc-cvs-stay-local, the latter will win out (which
won't necessarily happen if we just declare the former to be an obsolete
alias of the latter).
So it seems we're actually buying some extra compatibility here at the
cost of some complexity. Do we care about the above detail?
If yes, vc-stay-local should remain as it is now. If not, I'll gladly
rename it back to vc-cvs-stay-local, and create an obsolete alias.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#19548: VC changes under-documented, needlessly incompatible
2016-05-08 23:42 ` Dmitry Gutov
2016-05-13 21:05 ` Glenn Morris
2016-05-15 23:37 ` Dmitry Gutov
@ 2016-05-23 17:36 ` Eli Zaretskii
2016-05-23 22:49 ` Dmitry Gutov
2 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2016-05-23 17:36 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 19548, esr
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 9 May 2016 02:42:09 +0300
> Cc: "Eric S. Raymond" <esr@thyrsus.com>
>
> Looking at the commit that made this (185320a5), I see no reason for
> there to be vc-stay-local. Quite the opposite, the commit message says:
>
> "The CVS back end retaiin this machibery and the vc-stay-local
> configuration variable now only affects it."
>
> Why don't we remove vc-stay-local instead, and keep the appropriately
> named vc-cvs-stay-local?
Will that also fix the typos in the log message? ;-)
Anyway, I tend to agree to go back to vc-cvs-stay-local.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#19548: VC changes under-documented, needlessly incompatible
2016-05-15 23:37 ` Dmitry Gutov
@ 2016-05-23 17:37 ` Eli Zaretskii
2016-05-23 23:07 ` Dmitry Gutov
0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2016-05-23 17:37 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 19548, esr
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 16 May 2016 02:37:33 +0300
> Cc: "Eric S. Raymond" <esr@thyrsus.com>
>
> On the other hand, vc-cvs-stay-local-p contains this bit of code:
>
> (let* ((sym (vc-make-backend-sym 'CVS 'stay-local))
> (stay-local (if (boundp sym) (symbol-value sym) vc-stay-local)))
>
> which seems like it will ensure that if the user has customized both
> vc-stay-local and vc-cvs-stay-local, the latter will win out (which
> won't necessarily happen if we just declare the former to be an obsolete
> alias of the latter).
>
> So it seems we're actually buying some extra compatibility here at the
> cost of some complexity. Do we care about the above detail?
I'm not sure I see the gain, given that (AFAIU) CVS is the only
back-end for which this option is relevant.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#19548: VC changes under-documented, needlessly incompatible
2016-05-23 17:36 ` Eli Zaretskii
@ 2016-05-23 22:49 ` Dmitry Gutov
0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Gutov @ 2016-05-23 22:49 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 19548, esr
On 05/23/2016 08:36 PM, Eli Zaretskii wrote:
> Will that also fix the typos in the log message? ;-)
I'm sure the next version of Reposurgeon will do that, as well as hunt
down and correct all the respective messages in the mailing list archives.
> Anyway, I tend to agree to go back to vc-cvs-stay-local.
Cool.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#19548: VC changes under-documented, needlessly incompatible
2016-05-23 17:37 ` Eli Zaretskii
@ 2016-05-23 23:07 ` Dmitry Gutov
2016-05-24 15:34 ` Eli Zaretskii
0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Gutov @ 2016-05-23 23:07 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 19548, esr
On 05/23/2016 08:37 PM, Eli Zaretskii wrote:
> I'm not sure I see the gain, given that (AFAIU) CVS is the only
> back-end for which this option is relevant.
For the benefit of someone who customized vc-stay-local 10 years ago?
Not very compelling, I agree.
Does this patch have your blessing?
Someone should test it out, to be safe; the only CVS repo I have to
experiment on doesn't seem to work great with either version of the code
(but then, it's an old public checkout of the Samba repository).
diff --git a/lisp/vc/vc-cvs.el b/lisp/vc/vc-cvs.el
index 2dca708..a2499a2 100644
--- a/lisp/vc/vc-cvs.el
+++ b/lisp/vc/vc-cvs.el
@@ -121,7 +121,7 @@ vc-cvs-use-edit
:version "21.1"
:group 'vc-cvs)
-(defcustom vc-stay-local 'only-file
+(defcustom vc-cvs-stay-local 'only-file
"Non-nil means use local operations when possible for remote
repositories.
This avoids slow queries over the network and instead uses heuristics
and past information to determine the current status of a file.
@@ -131,11 +131,11 @@ vc-stay-local
all other VC operations.
The value can also be a regular expression or list of regular
-expressions to match against the host name of a repository; then VC
-only stays local for hosts that match it. Alternatively, the value
-can be a list of regular expressions where the first element is the
-symbol `except'; then VC always stays local except for hosts matched
-by these regular expressions."
+expressions to match against the host name of a repository; then
+vc-cvs only stays local for hosts that match it. Alternatively,
+the value can be a list of regular expressions where the first
+element is the symbol `except'; then vc-cvs always stays local
+except for hosts matched by these regular expressions."
:type '(choice (const :tag "Always stay local" t)
(const :tag "Only for file operations" only-file)
(const :tag "Don't stay local" nil)
@@ -789,8 +789,7 @@ vc-cvs-stay-local-p
individually should stay local."
(if (listp file)
(delq nil (mapcar (lambda (arg) (vc-cvs-stay-local-p arg)) file))
- (let* ((sym (vc-make-backend-sym 'CVS 'stay-local))
- (stay-local (if (boundp sym) (symbol-value sym) vc-stay-local)))
+ (let ((stay-local vc-cvs-stay-local))
(if (symbolp stay-local) stay-local
(let ((dirname (if (file-directory-p file)
(directory-file-name file)
^ permalink raw reply related [flat|nested] 11+ messages in thread
* bug#19548: VC changes under-documented, needlessly incompatible
2016-05-23 23:07 ` Dmitry Gutov
@ 2016-05-24 15:34 ` Eli Zaretskii
2016-05-25 1:06 ` Dmitry Gutov
0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2016-05-24 15:34 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 19548, esr
> Cc: rgm@gnu.org, 19548@debbugs.gnu.org, esr@thyrsus.com
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 24 May 2016 02:07:35 +0300
>
> Does this patch have your blessing?
Yes, it looks good to me.
> Someone should test it out, to be safe; the only CVS repo I have to
> experiment on doesn't seem to work great with either version of the code
> (but then, it's an old public checkout of the Samba repository).
You can try this one:
http://www.delorie.com/djgpp/cvs.html
Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#19548: VC changes under-documented, needlessly incompatible
2016-05-24 15:34 ` Eli Zaretskii
@ 2016-05-25 1:06 ` Dmitry Gutov
0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Gutov @ 2016-05-25 1:06 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 19548, esr
On 05/24/2016 06:34 PM, Eli Zaretskii wrote:
> Yes, it looks good to me.
Pushed, thanks. If someone would like to point out what else we're
missing in this bug, that would be great.
I've glanced through the "Changes from the pre-25.1 API" list, and one
change we could make to improve backward compatibility there, is to
catch wrong-number-of-arguments when calling the `diff' method, and
retry with one fewer argument.
Doesn't really seem worth the trouble, however. The list of changes is
longer than that, and third-party backends will have to adapt anyway.
Similarly, the change to dir-status-files could be smoothed over by
continuing to pass some dummy fourth argument, but it's unclear when
we'd be allowed to stop doing that. So maybe not doing that at all is
just as valid an option. IIRC, nobody has really complained about their
backend having stopped working.
> You can try this one:
>
> http://www.delorie.com/djgpp/cvs.html
Thanks. revert-buffer works *very* slowly with it using the default
settings, but at least the results look correct.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-05-25 1:06 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-09 17:41 bug#19548: VC changes under-documented, needlessly incompatible Glenn Morris
2016-05-08 23:42 ` Dmitry Gutov
2016-05-13 21:05 ` Glenn Morris
2016-05-14 7:41 ` Eli Zaretskii
2016-05-15 23:37 ` Dmitry Gutov
2016-05-23 17:37 ` Eli Zaretskii
2016-05-23 23:07 ` Dmitry Gutov
2016-05-24 15:34 ` Eli Zaretskii
2016-05-25 1:06 ` Dmitry Gutov
2016-05-23 17:36 ` Eli Zaretskii
2016-05-23 22:49 ` Dmitry Gutov
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).