[PATCH 6 of 8] dirstate: create class for status lists

[Martin von Zweigbergk]

On Mon, Oct 6, 2014 at 10:09 AM, Mads Kiilerich <mads at kiilerich.com> wrote:
> On 10/02/2014 07:39 AM, Martin von Zweigbergk wrote:
>>
>> On Wed, Oct 1, 2014 at 6:40 PM, Mads Kiilerich <mads at kiilerich.com> wrote:
>>>
>>> On 10/02/2014 02:01 AM, Martin von Zweigbergk wrote:
>>>>
>>>>    @@ -2511,19 +2511,19 @@
>>>>                                      unknown=True, ignored=True,
>>>> clean=True)
>>>>            else:
>>>>                changes = repo.status(match=m)
>>>> -            for kind in changes:
>>>> +            for kind in changes.all():
>>>>                    for abs in kind:
>>>>                        names[abs] = m.rel(abs), m.exact(abs)
>>>>                  m = scmutil.matchfiles(repo, names)
>>>>    -        modified = set(changes[0])
>>>> -        added    = set(changes[1])
>>>> -        removed  = set(changes[2])
>>>> -        _deleted = set(changes[3])
>>>> -        unknown  = set(changes[4])
>>>> -        unknown.update(changes[5])
>>>> -        clean    = set(changes[6])
>>>> +        modified = set(changes.modified)
>>>> +        added    = set(changes.added)
>>>> +        removed  = set(changes.removed)
>>>> +        _deleted = set(changes.deleted)
>>>
>>>
>>> Instead of refactoring an unused line,
>>
>> Are you referring to '_deleted'? Why do you think it's unused? Because
>> it starts with underscore? I didn't see anything about that in the
>> style guide, but I think I've seen that convention in some places.
>> Should I send a patch?
>
>
> Yes, it is in the default for pylint --dummy-variables-rgx and thus a
> somewhat common convention in Python ... but I guess you are right that it
> isn't in the Mercurial style guide.
>
> afead12e724b do however seem to use a very different convention - perhaps
> more inspired by how leading _ in namespaces usually indicate "private". It
> tricked me and is thus a bit dangerous ... either that, or it is me who are
> dangerous.

Ah, thanks. I didn't even check.

> I would prefer if we consistently could use the leading-underscore-is-unused
> convention.

I would at least agree that consistency is better. In the languages
I've used before, I would generally create a narrower scope by
extracting a method. If that makes sense in Python too, it seems like
underscore as prefix could be reserved for unused variables. It seems
like we'd have to convince at least marmoute.