bug#12159: 24.1.50; vc-dir: Need a way to hide unregistered files

[Stefan Monnier <monnier@iro.umontreal.ca>]

I'm really sorry you feel so frustrated.

All the process (from my point of view) is trying to get the patch to be
as simple and clean as it can be.  First by changing the actual
behavior, then by getting the details of the code right (hence my last
hunk-by-hunk comments).

This last message should have admittedly started with encouraging
words rather than dryly jumping into the nitpicking without even warning
that these are nitpicks.

Hopefully the end result should be a patch of barely 10 lines including
context, so while you may find it frustrating to go through this ordeal
for such a tiny change, I see it as a great success to bring down the
original request to such a simple change.

> Reviewers have infinite time to review the patch.

I don't know of any reviewer with infinite time, but if you find one,
please send him up here, we'd love to have one of those.


        Stefan


>>>>> "Jambunathan" == Jambunathan K <kjambunathan@gmail.com> writes:

> I wish reviewers provide feedback which is comprehensive right from the
> word go.  Let me explain ...

> When I submitted my patch it was complete i.e., I did not present it
> hunk-by-hunk.  I re-worked the patch based on feedback and I have
> demonstrated some seriousness in making the patch acceptable.

> Unfortunately, the review process here seems to have gone by "hunk by
> hunk" mode.  A small note here, a small note there.  For something as
> simple as this patch, why should we have 100 exchanges?

> I can't care less if you call my patch a crap or hold an opinion that I
> should never enter a programmer's territory.  It is not what I am
> talking about.

> Reviewers have infinite time to review the patch.  Let them collect
> their notes and give a comprehensive list of what they think is
> acceptable to them.

> I hope I am not placing an un-reasonable demand.  

> We are talking of an implicit social contract that reviewers and patch
> submitters should adhere to.  Unfortunately, it is only the patch
> submitters end of the contract that gets much emphasis.

> Jambunathan K.

>>> +	* vc/vc-dir.el (vc-dir-hide-these-states): New custom variable.
>> 
>> Don't bother.  Just always default to up-to-date.
>> 
>>> +(defun vc-dir-hide-some-states (&optional states)
>> 
>> Make it `state' and not a list.
>> 
>>> +  (interactive
>>> +   ;; Interactive use.
>> 
>> Redundant comment.
>> 
>>> +  ;; Non-interactive use.
>>> +  (unless (called-interactively-p 'any)
>>> +    (setq states (or states vc-dir-hide-these-states)))
>> 
>> The test is wrong (it prevents non-interactive use where you specify
>> the state explicitly).
>> The above should simply be (unless state (setq state 'up-to-date)).
>> 
>>> +(defun vc-dir-hide-up-to-date ()
>>> +  "Hide up-to-date items from display."
>>> +  (interactive)
>>> +  (vc-dir-hide-some-states '("up-to-date")))
>> 
>> Why bother?
>> 
>> 
>> Stefan