Option to exclude features

[wlupton]

The --feature option gives a list of features that are to be considered enabled (by default all features are considered enabled, but if this option is present then only the listed features are considered enabled).

Would there any be support for adding an --exclude-features (or similar) option that would exclude the specified features?

A use case for this would be to test a specific feature – maybe one that has just been added – to ensure that nothing has been forgotten, such as if-featuring leafref nodes, and do this quickly on the command line without having to go and copy/paste all other features that should remain enabled.

If people think that this is a good idea then I'd be happy to contribute a PR.

Note:

• It wouldn't really make sense for both --features and --exclude-features to be specified, but if they were both specified then I guess the latter would operate on the features selected by the former.
• I assume (but haven't checked) that pyang already does the right thing with features that are dependent on other features, e.g., if feature XXX has an if-feature YYY then presumably enabling XXX will automatically enable YYY (and, with the new proposed option, enabling XXX and then attempting to exclude YYY should be an error)

[mbj4668]

Hi,

I think such a feature makes sense.

Den mån 6 dec. 2021 kl 16:25 skrev William Lupton @.***

:

The --feature option gives a list of features that are to be considered enabled (by default all features are considered enabled, but if this option is present then only the listed features are considered enabled).

Would there any be support for adding an --exclude-features (or similar) option that would exclude the specified features?

A use case for this would be to test a specific feature – maybe one that has just been added – to ensure that nothing has been forgotten, such as if-featuring leafref nodes, and do this quickly on the command line without having to go and copy/paste all other features that should remain enabled.

If people think that this is a good idea then I'd be happy to contribute a PR.

Note:

• It wouldn't really make sense for both --features and --exclude-features to be specified, but if they were both specified then I guess the latter would operate on the features selected by the former.

I think an error if both are given is best.

• I assume (but haven't checked) that pyang already does the right thing with features that are dependent on other features, e.g., if feature XXX has an if-feature YYY then presumably enabling XXX will automatically enable YYY (and, with the new proposed option, enabling XXX and then attempting to exclude YYY should be an error)

Yes it does the right thing (but not the same right thing that you think ;) Seriously, it doesn't do any auto-enabling of features. It will enable the features you ask it to enable.

/martin

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mbj4668/pyang/issues/779, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABJBUQLH5KRTCWYE5HONUDUPTIVRANCNFSM5JO4WKCQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[wlupton]

Thanks @mbj4668. Thinking about the implementation, I think we need to present an unchanged interface to plugins, so I'm thinking this:

• As you recommend, only one of --features and --exclude-features is permitted.
• When processing arguments the current logic is shown below, and I think we can do something very similar for --exclude-features, still putting the results in ctx.features but prefixing feature names with a special character (!?). Alternatively add another ctx field but I think I prefer to keep all the info in ctx.features (and to keep its entries as strings).

  for f in ctx.opts.features:
      (modulename, features) = parse_features_string(f)
      ctx.features[modulename] = features

• Update Statement.has_feature(name) to work with the above (it's the only place, apart from the pyang executable and plugins, where ctx.features is used)
• I think we can probably safely move the verify the given features block to just before transforms are invoked? Add logic to replace !feature with a list of all features except for the excluded features. This means that all plugins will see an unchanged interface.
• If we can't move the verify the given features block as proposed then there will still need to be a check after transforms are invoked, just in case a transform removed a feature (very unlikely!).

Thoughts?

[wlupton]

Does the suggested implementation look OK to you @mbj4668?

[mbj4668]

This seems reasonable. And I agree that we can move the verify the given features block as you suggest.