Skip to content
This repository has been archived by the owner on Jun 17, 2020. It is now read-only.

Add a thing to make using BufferCollator as some sort of queue easy and ... #4

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

alex
Copy link
Owner

@alex alex commented Jul 7, 2014

...fast

@@ -187,9 +187,16 @@ API Reference

Adds the contents of a view to the collator.

.. method:: collapse()
.. method:: collapse(max_bytes=-1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Below you called it max_length.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhh, where?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. In the imlementation.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed.

@@ -453,16 +452,29 @@ def append(self, view):
self._views.append(view)
self._total_length += len(view)

def collapse(self):
def collapse(self, max_bytes=-1):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not None instead of a sentinel value?

(ok, so technically None is a sentinel value, but at least it is of a different type)

@radix
Copy link
Contributor

radix commented Jul 18, 2014

Also, have you considered making collapse a non-destructive operation? it could just create a buffer and leave the old stuff there. You'd need to allow passing a start index in addition to a max_bytes, though.

@alex
Copy link
Owner Author

alex commented Jul 19, 2014

@dreid @radix: Reviews responded to.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants