Ticket #5384 enhancement closed fixed

Opened 2 years ago

Last modified 2 years ago

Provide a library for bitvector-like valued named constants

Reported by: exarkun Owned by: exarkun
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch: branches/flag-constants-5384
(diff, github, buildbot, log)
Author: exarkun Launchpad Bug:

Description

#4671 proposed adding a lot of new symbolic constant features to a helper somewhere in Twisted. This is for the kind of symbolic constant that has a value like a bit in a bitvector and which can be combined from other constants which are part of the same logical group, using all of the traditional bitwise operations.

This would be useful in conch, because SSH uses lots of bitvector-style constants.

Change History

1

Changed 2 years ago by exarkun

  • branch set to branches/flag-constants-5384
  • branch_author set to exarkun

(In [33419]) Branching to 'flag-constants-5384'

2

Changed 2 years ago by exarkun

  • keywords review added
  • owner exarkun deleted

I implemented many features. Possibly there is room for more, but maybe this is a good self-contained start? One thing that I'm somewhat uncertain about is the behavior of (SomeFlags.A | SomeFlags.B) is (SomeFlags.A | SomeFlags.B) or perhaps even more interestingly, SomeFlags.A is (SomeFlags.A | SomeFlags.B ^ SomeFlags.B) (in contrast to SomeFlags.A is SomeFlags.A).

Also, I feel like the documentation is terribly repetitive.

 Build results

Finally, this is the last ticket for #4671, so when I close this ticket I expect to close that ticket as well.

3

Changed 2 years ago by glyph

The buildbots are in a pretty nasty state, and the patch is a bit large, but I like what I see so far. I will think about it some more and hopefully look at a more comprehensive list of build results when the builders are back online.

4

Changed 2 years ago by therve

  • keywords review removed
  • owner set to exarkun
  • +    Tests for L{twisted.python.constdants.Flags}, a base class for containers of
    

Typo: constants.

  • Flags and FlagConstant should be added to constants.__all__.
  • +        result._realize(self._container, set([]), 0)
    

I don't think you need to pass anything to build an empty set.

  • +        bits set which were not set in the original constant.  constants.
    

There is an extra "constants" here.

  • It may be more interesting to also test READ in FlagConstantNegationTests.test_value, as otherwise you don't check completely the value passed in the realize call in invert.
  • +            if (flag.value & self.value) == 0:
    

You don't need the parenthesis here.

Great branch, please merge if the buildot is happy.

5

Changed 2 years ago by exarkun

  • status changed from new to closed
  • resolution set to fixed

(In [33677]) Merge flag-constants-5384

Author: exarkun Reviewer: therve Fixes: #5384, #4671

Add bitvector-like constant support to twisted.python.constants.

Note: See TracTickets for help on using tickets.