Bug reports for GNU Emacs, the Swiss army knife of text editors

Jambunathan K | 8 Aug 2012 20:07

Picon

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


I have a tendency to accumulate unregistered files in my vc-dir work
area before I do away with all of them with "git clean -d -f".

It will be convenient if the existing `vc-dir-hide-up-to-date' (and the
"x" binding) is extended so that files in arbitrary state - including
unregistered - could be hidden.

I have posted my patch to emacs-devel.  See following thread:
    http://lists.gnu.org/archive/html/emacs-devel/2012-08/msg00277.html

With my patch, I can accomplish what I want with

    (custom-set-variables
     '(vc-dir-hide-these-states (quote ("unregistered" "up-to-date"))))

Feel free to close this patch once the above patch is suitably dealt
with.

In GNU Emacs 24.1.50.22 (i686-pc-linux-gnu, GTK+ Version 2.20.1)
 of 2012-08-08 on debian-6.05
Bzr revision: 109513 kjambunathan <at> gmail.com-20120808150232-cvekujrpotm9qlwa
Windowing system distributor `The X.Org Foundation', version 11.0.10707000
Important settings:
  value of $LANG: en_IN
  locale-coding-system: iso-latin-1-unix
  default enable-multibyte-characters: t

Glenn Morris | 8 Aug 2012 20:27

Picon

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


http://debbugs.gnu.org/cgi/bugreport.cgi?bug=6148

Jambunathan K | 8 Aug 2012 20:52

Picon

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

Glenn Morris <rgm <at> gnu.org> writes:

> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=6148

Quick comments:

1. My patch could make use of completing-read-multiple.  Currently it
   uses completing-read.  So one has to do multiple "C-u x"s.

2. Magnus' changes assumes that states are just elisp symbols.  This may
   not be true.  In case of locked states - designated as USER in
   `vc-state' - the state could actually be a string.

Stefan Monnier | 9 Aug 2012 03:54

Picon

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

> 1. My patch could make use of completing-read-multiple.  Currently it
>    uses completing-read.  So one has to do multiple "C-u x"s.

Maybe a better option is to just have "C-u x" hide all files in the same
state as the file-at-point.  So you still need several "C-u x"s but you
don't need to enter the states, you can just point to them.

        Stefan

Jambunathan K | 11 Aug 2012 19:41

Picon

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

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

>> 1. My patch could make use of completing-read-multiple.  Currently it
>>    uses completing-read.  So one has to do multiple "C-u x"s.
>
> Maybe a better option is to just have "C-u x" hide all files in the same
> state as the file-at-point.  So you still need several "C-u x"s but you
> don't need to enter the states, you can just point to them.

Currently "x" acts buffer-wide.

Your suggestion is to hide the files that are in the same state as the
file at point.  

If there are marked files, then potentially a super-set of marked files
could get hidden.  This "superset" behaviour will surprise the user -
"Why does even un-marked files get hidden?"

I am little bit hesitant to re-work the patch.

>         Stefan

Stefan Monnier | 12 Aug 2012 00:40

Picon

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

> If there are marked files, then potentially a super-set of marked files
> could get hidden.  This "superset" behaviour will surprise the user -
> "Why does even un-marked files get hidden?"

I don't see why it should pay attention to the marks.

        Stefan

Jambunathan K | 12 Aug 2012 03:50

Picon

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

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

>> If there are marked files, then potentially a super-set of marked files
>> could get hidden.  This "superset" behaviour will surprise the user -
>> "Why does even un-marked files get hidden?"
>
> I don't see why it should pay attention to the marks.

I am attaching the modified patch.

Attachment (vc-dir-hide-some-states.patch): text/x-diff, 5042 bytes

Andreas Schwab | 12 Aug 2012 09:37

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

Jambunathan K <kjambunathan <at> gmail.com> writes:

> +(defcustom vc-dir-hide-these-states '("up-to-date")
> +  "States hidden by `vc-dir-hide-some-states'."
> +  :type '(choice
> +	  (const :tag "None")
> +	  (set :tag "Choices"
> +	       (string :tag "VC State" "added")

Why a string?  Using the symbol would be more natural.

Andreas.

--

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Jambunathan K | 12 Aug 2012 11:58

Picon

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

Andreas Schwab <schwab <at> linux-m68k.org> writes:

> Jambunathan K <kjambunathan <at> gmail.com> writes:
>
>> +(defcustom vc-dir-hide-these-states '("up-to-date")
>> +  "States hidden by `vc-dir-hide-some-states'."
>> +  :type '(choice
>> +	  (const :tag "None")
>> +	  (set :tag "Choices"
>> +	       (string :tag "VC State" "added")
>
> Why a string?  Using the symbol would be more natural.

There is a note up in the thread, which reads:

,----
| 2. Magnus' changes assumes that states are just elisp symbols.  This
| may not be true.  In case of locked states - designated as USER in
| `vc-state' - the state could actually be a string.
`----

I have gone by what I could make out from the dav backend.

Btw, does it make a big difference?

> Andreas.

Andreas Schwab | 12 Aug 2012 12:02

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

Jambunathan K <kjambunathan <at> gmail.com> writes:

> Andreas Schwab <schwab <at> linux-m68k.org> writes:
>
>> Jambunathan K <kjambunathan <at> gmail.com> writes:
>>
>>> +(defcustom vc-dir-hide-these-states '("up-to-date")
>>> +  "States hidden by `vc-dir-hide-some-states'."
>>> +  :type '(choice
>>> +	  (const :tag "None")
>>> +	  (set :tag "Choices"
>>> +	       (string :tag "VC State" "added")
>>
>> Why a string?  Using the symbol would be more natural.
>
> There is a note up in the thread, which reads:
>
> ,----
> | 2. Magnus' changes assumes that states are just elisp symbols.  This
> | may not be true.  In case of locked states - designated as USER in
> | `vc-state' - the state could actually be a string.
> `----

There is nothing wrong with using a string where the state is a string.
But if the user name happens to match the name of one of the state
symbols it becomes ambigous.

Andreas.

--

--

(Continue reading)

Jambunathan K | 12 Aug 2012 13:04

Picon

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

Andreas Schwab <schwab <at> linux-m68k.org> writes:

> Jambunathan K <kjambunathan <at> gmail.com> writes:
>
>> Andreas Schwab <schwab <at> linux-m68k.org> writes:
>>
>>> Jambunathan K <kjambunathan <at> gmail.com> writes:
>>>
>>>> +(defcustom vc-dir-hide-these-states '("up-to-date")
>>>> +  "States hidden by `vc-dir-hide-some-states'."
>>>> +  :type '(choice
>>>> +	  (const :tag "None")
>>>> +	  (set :tag "Choices"
>>>> +	       (string :tag "VC State" "added")
>>>
>>> Why a string?  Using the symbol would be more natural.
>>
>> There is a note up in the thread, which reads:
>>
>> ,----
>> | 2. Magnus' changes assumes that states are just elisp symbols.  This
>> | may not be true.  In case of locked states - designated as USER in
>> | `vc-state' - the state could actually be a string.
>> `----
>
> There is nothing wrong with using a string where the state is a string.
> But if the user name happens to match the name of one of the state
> symbols it becomes ambigous.

Sure, I will name my next kid `conflict' or even better `missing'.  In

(Continue reading)

Andreas Schwab | 12 Aug 2012 16:07

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

Jambunathan K <kjambunathan <at> gmail.com> writes:

> Sure, I will name my next kid `conflict' or even better `missing'.

<http://xkcd.com/327/>

Andreas.

--

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Stefan Monnier | 12 Aug 2012 16:13

Picon

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

> +	* 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

(Continue reading)

Jambunathan K | 12 Aug 2012 16:22

Picon

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


I am not working on this patch any more.

Jambunathan K | 12 Aug 2012 21:11

Picon

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


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.
>

(Continue reading)

Eli Zaretskii | 12 Aug 2012 21:20

Picon

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

> From: Jambunathan K <kjambunathan <at> gmail.com>
> Date: Mon, 13 Aug 2012 00:41:00 +0530
> Cc: 12159 <at> debbugs.gnu.org
> 
> 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.

I'm sorry you feel this way.  However, after reading the entire

(Continue reading)

Jambunathan K | 12 Aug 2012 21:40

Picon

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

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Jambunathan K <kjambunathan <at> gmail.com>
>> Date: Mon, 13 Aug 2012 00:41:00 +0530
>> Cc: 12159 <at> debbugs.gnu.org
>> 
>> 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.

(Continue reading)

Bastien | 13 Aug 2012 16:21

Picon

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

Jambunathan K <kjambunathan <at> gmail.com> writes:

> (I remember my troubles making even limited progress with
> Ulysess, Portrait of artist as an young man or the Sound and Fury.)

Parsing these great novels is like fixing dependancies in a messy
library: you constantly need to zoom in and out, switch from the local
evaluation of the meaning to the global interpretation context, try to
synchronize your flow of consciousness to the narrative pace, while
enjoying the whole experience.

Perhaps the emacswiki style is just training for more sweaty days :)

--

-- 
 Bastien

Stefan Monnier | 13 Aug 2012 00:35

Picon

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

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 <at> 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

(Continue reading)

Jambunathan K | 13 Aug 2012 19:46

Picon

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


FWIW, I am attaching two patches.  If you want any changes please do it
yourself.

patch-1 :: There is only one command 'x' - which hides state at point.
patch-2 :: `x' hides up-to-date and `C-u x' hides state at point.

Personally, I will go with patch 1.  It is simpler.  No prefix key is
used.

Btw, reviewer who takes infinite time to review could either be a
perfectionist or a procrastinator .

Attachment (bug#12159-approach-1.patch): text/x-diff, 3968 bytes

Attachment (bug#12159-approach-2.patch): text/x-diff, 2721 bytes


>> +	* 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.

(Continue reading)

Stefan Monnier | 13 Aug 2012 23:34

Picon

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

> patch-1 :: There is only one command 'x' - which hides state at point.
> patch-2 :: `x' hides up-to-date and `C-u x' hides state at point.

Thanks.

> Personally, I will go with patch 1.  It is simpler.  No prefix key is
> used.

I can agree that patch 1 is simpler and might be preferable, but given
the pre-existence of `x', I'd rather preserve its behavior.
Furthermore, I do use `x' fairly often and usually without paying much
attention to where I am.

So, I installed patch 2.

> Btw, reviewer who takes infinite time to review could either be a
> perfectionist or a procrastinator .

Indeed, from the submitter's point of view, they're indistinguishable.
Usually, if I'm the reviewer and the review isn't coming back quickly
... well let's just say it's not because I'm a perfectionist.

        Stefan

Bastien | 14 Aug 2012 00:09

Picon

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

Stefan Monnier <monnier <at> IRO.UMontreal.CA> writes:

> ... well let's just say it's not because I'm a perfectionist.

Hopefully enough, even perfect procrastinators can get 
some stuff done:

  http://structuredprocrastination.com

--

-- 
 Bastien

Jambunathan K | 14 Aug 2012 06:28

Picon

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

Glenn Morris <rgm <at> gnu.org> writes:

> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=6148

Please mark both these bugs as fixed in bzr builds.

Return

Return to gmane.emacs.bugs.

Search Archive

Language

Change language

Options

Current view: Threads only / Showing only 20 lines / Not hiding cited text.
Change to All messages, whole messages, or hide cited text.

Post a message
NNTP Newsgroup
Classic Gmane web interface
XML

XML

RSS Feed
List Information

About Gmane