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

From: Jambunathan K <[email-masked]>
Date: Mon, 13 Aug 2012 00:41:00 +0530

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