Skip to content
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

OutputArea.append_type functions are not prototype methods #5295

Merged
merged 2 commits into from Mar 10, 2014

Conversation

minrk
Copy link
Member

@minrk minrk commented Mar 8, 2014

These are registered in OutputArea.append_map. Overriding them requires only replacing keys in this dict. Prototype methods imply that overriding the prototype will change behavior, which is not true.

@minrk
Copy link
Member Author

minrk commented Mar 8, 2014

looks like there's something funny about the js context in which the tests run. Will investigate.

"text/latex" : OutputArea.prototype.append_latex,
"application/javascript" : OutputArea.prototype.append_javascript,
"application/pdf" : OutputArea.prototype.append_pdf
"text/plain" : append_text,
Copy link
Member

Choose a reason for hiding this comment

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

just as a quick glance append_stream uses this.append_text, so either fix there or keep this one attached to output area?

@ivanov
Copy link
Member

ivanov commented Mar 8, 2014

PR looks fine and makes sense otherwise.

@ellisonbg
Copy link
Member

I am confused, most of these methods use this. How does this PR not break things?

@minrk
Copy link
Member Author

minrk commented Mar 10, 2014

Entries in append_map are called via append.apply(this, [value, metadata, element]). They are never treated as prototype methods. This is how we achieve extensible mimetype support.

@ellisonbg
Copy link
Member

OK, thanks for the clarification. I have looked at this and tested it interactively. Looks fine, merging.

ellisonbg added a commit that referenced this pull request Mar 10, 2014
OutputArea.append_type functions are not prototype methods
@ellisonbg ellisonbg merged commit 18300d5 into ipython:master Mar 10, 2014
@minrk minrk deleted the no-append-prototype branch March 31, 2014 23:36
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
OutputArea.append_type functions are not prototype methods
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants