New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revamp content views #1662

Open
cortesi opened this Issue Oct 24, 2016 · 3 comments

Comments

Projects
None yet
3 participants
@cortesi
Member

cortesi commented Oct 24, 2016

The way we deal with content views at the moment is sub-optimal. We're caching generated urwid.Text objects in mitmproxy console, and we have no good way for views to be dynamically constructed on demand. To fix this, I propose that:

  • We change the flow body view to use an urwid ListWalker, which constructs the urwid.Text instances on demand.
  • We define a content view function as returning something that implements collections.abc.Sequence. Simple views will just return a list, while more complex views can return an object that constructs data on demand. More complex objects are also free to progressively save internal state for expensive data.
  • Mitmproxy console can now use an LRU cache to cache objects returned by content views, using the view type and data as keys. These hash more naturally than entire Message objects which is what we use at the moment. For expensive views, the previously constructed view object will be returned from the cache with all of the dynamically computed data intact.
@cortesi

This comment has been minimized.

Member

cortesi commented Oct 24, 2016

As a corollary there, we should also clean up the content view structure a bit. We have a TON of content views, and they’re growing in number. Usually only a subset is relevant to a particular message, and we can see what is probably relevant based on the headers. I’d like to change things so that we only prompt the user for views that make sense for the current flow, with an a) for all option that takes you to a full list of all content views for exceptional circumstances. So, for example, there’s no point prompting for the image flow view if we can see the flow is json.

@mhils

This comment has been minimized.

Member

mhils commented Dec 10, 2016

We should also define on what input a contentview is working. The current approach is to have content as bytes and metadata, so that we can also use them for WebSocket messages.
For the HTML/XML views we ideally need str, not bytes (i.e. message.text, not message.content). Do we want to pass that in as metadata or do we have a better idea?

@mhils

This comment has been minimized.

Member

mhils commented Dec 10, 2016

The review comments in #1831 are relevant to this.

@cortesi cortesi modified the milestones: 2.0, 1.0 Dec 15, 2016

@Kriechi Kriechi modified the milestones: 2.0, 3.0 Feb 21, 2017

@mhils mhils modified the milestone: 3.0 Mar 8, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment