Support new --exclude-features option

[wlupton]

Also update the man page and add some tests.

See issue #779 for context.

[wlupton]

I suddenly realised that there's a problem. The logic that replaces !feature exclusions with feature inclusions is plain wrong. I should have set up a better test. Please don't merge the PR yet. I'll fix it...

Update: I believe that this problem has been fixed (and that the tests will now catch this).

[wlupton]

@mbj4668 , thanks for the feedback. Let's keep discussing. Also, I wanted to mention a couple of things:

single-character option?

--hello has -L and --features has -F. Could/should --exclude-features have a single-character option? If so, what? -X (for 'exclude')? -G (because it comes after F)? My vote would be for -X (first syllable of exclude), which goes nicely with -L (first syllable of Cockney 'ello).

Note: I think that the man page doesn't mention -F?

hello

When I started implementing the new feature, I hadn't fully appreciated the interaction with --hello. I think that this (in the man page) is confusing (although correct):

This option can be given multiple times, and may be also combined with --hello. If a --features option specifies
different supported features for a module than the hello file, the --features option takes precedence.

I think it's the word different that's confusing. The fact is that --features (for a given module) overrides --hello regardless of whether it specifies different features (yes this only has any effect if the feature-sets are different).

In the end I added the equivalent text for --exclude-features (having initially thought that its description didn't need to mention --hello). I felt that was OK because "specifies different features" is quite vague and could be taken to mean "results in different features".

But now that both --features and --exclude-features can both be specified, perhaps we should rethink how we explain this. Conceptually I think we have this timeline (for each module):

1. All features are initially selected
2. A --hello option may replace all of the selected features
3. A --features option may replace all of the selected features
4. An --exclude-features option may remove specified selected features

[mbj4668]

I think -X would be fine. If you fix this, it would be great if you fixed -f in the man page as well!

I agree that the wording with "different" is a bit confusing. Feel free to fix this text as well!

[wlupton]

Great! Leave it all with me. Sorry it's taken so much discussion, but I hope that we'll all be happy with the result.

[wlupton]

OK. I've made the changes. A few notes:

• It's not an error to specify --exclude-features with an empty list of modules. There's no reason to do this, but I felt that it was cleaner (and more symmetrical) to permit it
• You can never combine --hello and --exclude-features for a given module, because hello will set ctx.features for the module, so --exclude-features isn't permitted for it. Is this OK or is this a case where it would make sense to allow --exclude-features to exclude features selected via --hello?
• I had earlier misunderstood (again) what you meant about keeping the code that sets ctx.features for plugins (so what I said about theoretical backwards-incompatibility was wrong). With the current code, if (for a given module) ctx.features is set then ctx.exclude_features is also set, and if ctx.exclude_features is set (even if the list of excluded features is empty) then ctx.features is set. The result is that plugins either see both None (indicating that neither option was specified) or both defined (indicating that one or other option was specified)

[mbj4668]

This looks ready to merge. Is it ready?

[wlupton]

I believe so.