[Aptitude-devel] Improvement/simplification of the implementation of parsing sorting policies…

[Manuel A. Fernandez Montecelo]

2012/2/27 Daniel Hartwig <mandyke at gmail.com>:
> Even though no existing sorting policies made use of arguments and
> removing them has made it easier for you to make use of tokenizer, it
> is worth keeping the feature:
>
>  - the user manual states sorting policies share the same syntax
>    as grouping policies, which permit and use arguments;
>  - it is not causing any problems at the moment;
>  - future sorting policies may make use of it: consider a by-arch
>    policy which permits the user to specify some important arch(s)
>    to sort before others;
>  - if this is added back in the future (very likely), then the
>    translations of messages ("By-foo sorting polices takes no
>    arguments.") will have been lost in the meantime, generating
>    extra work for translaters.
>
> Had the original parser been problematic, I'd say this was a good
> choice (remove the problems and some disused features).  But since
> it was not problematic it is prefered to leave the code as it is,
> with the potentially useful features in place.
>
> Lastly, the ideal, final outcome outcome of any refactoring of the
> sorting or grouping policy parsers is to unite these two pieces of
> code since the format is identical between them.  There is this
> comment by D. Burrows to back up this statement:
>
> -- load_sortpolicy.cc
> //  Loads sorting policies -- similar to grouping policies.  (in fact, I did
> // this with cut&paste, and think maybe I should consider trying to share
> // code between the two parsers..)

I disagree.  So the way to produce the original code is fundamentally
the wrong way to do it, acknowledged by the original author.  However
this wasn't fixed for 6 years.  The whole file was unchanged since
those days, no new policy was introduced whatsoever in the ~6 years.

> D. Burrows has been working on making improvements to this code,
> including making it more generic.  This was being developed in
> load_grouppolicy.cc ("git log src/load_grouppolicy.cc").  That code is
> a fair bit more complicated, but any attempt to bring these two
> parsers together should definitely start from there instead of
> load_sortpolicy.cc.

My change, stripping the sorting policies and leaving a trivial amount
of mostly empty classes and trivial parser, actually paves the way for
refactoring the whole bunch of files to use a common method, seeing
that the conversion to anything else is equally trivial.

But I guess that you disagree, so sorry for that.

> For these reasons alone I seriously think it is worth reverting this
> change completely.

OK, go ahead then.

> If you have not done so already, I would strongly suggest becoming
> familiar with all of those files before continuing to develop major
> changes in aptitude.

OK, I will refrain from develop major changes in aptitude then.

Cheers.