[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#7675: 24.0.50; vc-checkin should prompt if the fileset changes

From: Stefan Monnier
Subject: bug#7675: 24.0.50; vc-checkin should prompt if the fileset changes
Date: Sun, 19 Dec 2010 21:50:03 -0500
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux)

>    Thanks for submitting a patch to try and fix this problem.
>    A quick obvious issue, tho:

>> --- a/lisp/vc/vc.el
>> +++ b/lisp/vc/vc.el
>    [...]
>> +(defun log-edit-deduce-fileset (state-model-only-files)

>    If it's in vc.el it can't start with the "log-edit-" prefix.

> Yes, I was wondering about that.  I had thought about putting it in
> log-edit.el, but it seems closer to VC internals than log-edit
> internals.

log-edit deals with editing the text buffer, so clearly this job is not
about log-edit but VC.

> Which would you prefer:  Rename it, or move it?  If
> "rename", would "vc-log-edit-deduce-fileset" suffice?

I guess so, tho I don't understand why you need this function.  IIUC you
should switch to vc-parent before calling vc-deduce-fileset, so that
function never needs to handle log-edit buffers.

>    And while I'm thinking of it, a VC fileset really ought to be
> represented by a struct, since all this car-ing and cdr-ing is really
> messy.  But filesets are passed to backends, so testing such a change
> would be a pain.  What are your thoughts?  It is worth it?

It doesn't seem complex enough to warrant such a change.
Maybe another approach is to use pcase.
E.g. instead of

   (let ((backend (car fs))
         (files (cdr fs)))

or something like that, you'd write

   (pcase-let ((`(,backend . ,files) fileset))

Experience with ML-style languages tends to indicate that such pattern
matching tends to reduce the need for named fields, tho obviously type
checking (and inference) is also an important part of it since it
catches incorrect patterns.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]