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

Expand a bit the documentation about transformers. #10391

Merged
merged 3 commits into from Mar 22, 2017

Conversation

Carreau
Copy link
Member

@Carreau Carreau commented Mar 10, 2017

Mostly answering questions I had myself while trying to write some stuff
and while trying to get a simple example working.

Mostly answering questions I had myself while trying to write some stuff
and while trying to get a simple example working.
@Carreau Carreau added this to the 6.0 milestone Mar 10, 2017
@@ -687,7 +686,6 @@ def push(self, lines):
"""

# We must ensure all input is pure unicode
lines = cast_unicode(lines, self.encoding)
Copy link
Member

Choose a reason for hiding this comment

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

Why is this changed in a doc PR? If you're sure removing it is OK, please remove the comment too.

:class:`~IPython.core.inputtransformer.Inputtransformer` and overwriting the
``push`` and ``reset`` methods. The reset method should send the full block of
transformed text. As an example a transformer the reversed the lines from last
to first.
Copy link
Member

Choose a reason for hiding this comment

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

We should say that this will break the automatic 'is this input complete' detection in console interfaces - with a transformer like this added, it will always think more input is needed, and you will need to use a shortcut to force it to execute.

This code in IPython strips a constant amount of leading indentation from each
line in a cell::

from IPython.core.inputtransformer import CoroutineInputTransformer
Copy link
Member

Choose a reason for hiding this comment

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

Do you want to add the import to your new example above too?

You may use :meth:`CoroutineInputTransformer.wrap` to simplify the creation of
such a transformer.

Here is a simple :class:`CoroutineInputTransformer` that can be though of be
Copy link
Member

Choose a reason for hiding this comment

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

s/though/thought

Copy link
Member

Choose a reason for hiding this comment

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

be

.. warning::

Transforming a full block at once will break the automatic detection of
wether a block of code is complete in interface relying on this
Copy link
Member

Choose a reason for hiding this comment

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

  • whether
  • interfaces

Copy link
Member Author

Choose a reason for hiding this comment

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

done.


Transforming a full block at once will break the automatic detection of
wether a block of code is complete in interface relying on this
functionality, as for example terminal IPython. You will need to use a
Copy link
Member

Choose a reason for hiding this comment

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

'as for example' doesn't work in English. You could use 'such as', 'including', or 'for example'.

('such as, for example, foo' is valid, but it's too verbose to read nicely here, IMO)

Copy link
Member Author

Choose a reason for hiding this comment

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

done,

@takluyver takluyver merged commit 226bd07 into ipython:master Mar 22, 2017
@Carreau Carreau deleted the redox branch May 7, 2017 05:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants