Replace usage of twisted.python.text and deprecate it

[Thijs Triemstra]

twisted.python.text can be deprecated after replacing usage of it.

This ticket covers the actual deprecation and replacement in core, and can be closed once these subtasks for the subprojects have been completed:

• conch (#6543)
• words (#6544)
• mail (#6545)
• lore (#6546)

[Thijs Triemstra]

(In [37342]) Branching to 'py3-text-6341'

[Thijs Triemstra]

(In [37344]) port t.p.text, add news file. refs #6341

[Jean-Paul Calderone]

Or uses of it could be replaced by textwrap and twisted.python.text could be deprecated and eventually removed.

[Thijs Triemstra]

Replying to exarkun:

Or uses of it could be replaced by textwrap and twisted.python.text could be deprecated and eventually removed.

Sounds good. Let's port it (which was quite simple) and file new tickets to replace usage and deprecate it. I'd like to see it's supported on py3 until it's removed in x releases. Current coverage is 93%.

Forced a new ​build, please review.

[Jean-Paul Calderone]

Let's port it (which was quite simple) and file new tickets to replace usage and deprecate it

I appreciate that you've already done some of the porting work, but as a general plan, this doesn't sound great. Why do more work instead of less work?

[Thijs Triemstra]

Alright, if you put it that way. I thought there might've been something with value in there.. :)

1. Couldn't find any usage of stringyString, isMultiline, endsInNewline, and removeLeadingBlanks, these can be deprecated directly.
2. greedyWrap is used once:

   ./twisted/conch/insults/window.py:599:                wrappedLines = tptext.greedyWrap(inputLines.pop(0), width)
   

and it's alias wordWrap twice in twisted/python/usage.py

1. removeLeadingTrailingBlanks is used quite a lot, but only, in lore..
2. splitQuoted is used in mail, python, words.. quite a alot.
3. strFile is used twice in twisted/mail/imap4.py

So completely deprecating the module isn't possible without doing the work listed above.

[Jean-Paul Calderone]

greedyWrap is just textwrap.wrap.

splitQuoted is just shlex.split.

At first glance, removeLeadingTrailingBlanks looks really, *really* like str.strip.

strFile could be made into a private helper for imap4.py.

[Thijs Triemstra]

Thanks for the review ;) Updating ticket title and description.

[Thijs Triemstra]

(In [37349]) replace usage of t.python.text, add news files. refs #6341

[Thijs Triemstra]

Replying to exarkun:

greedyWrap is just textwrap.wrap.

Replaced.

splitQuoted is just shlex.split.

Replaced.

At first glance, removeLeadingTrailingBlanks looks really, *really* like str.strip.

It's used here:

./twisted/lore/tree.py: data = cStringIO.StringIO(text.removeLeadingTrailingBlanks(data)) ./twisted/lore/texi.py: self.writer(text.removeLeadingTrailingBlanks(buf.getvalue())) ./twisted/lore/slides.py: data = text.removeLeadingTrailingBlanks(data) ./twisted/lore/latex.py: self.writer(text.removeLeadingTrailingBlanks(buf.getvalue())) ./twisted/lore/latex.py: self.writer(text.removeLeadingTrailingBlanks('\n'.join(lines)))

Any suggestions for a better version are welcome but made it a private helper for lore as lore.tree.removeLeadingTrailingBlanks.

strFile could be made into a private helper for imap4.py.

Added it as imap4._strFile.

[Thijs Triemstra]

Added a deprecation warning in twisted.python, similar to the deprecations in twisted.scripts:

$ python -3 -c "from twisted.python import text"
-c:1: DeprecationWarning: twisted.python.text was deprecated in Twisted 13.1.0: text has been deprecated.

Forced a new ​build, up for review.

[Tom Prince]

1. The following doesn't emit a deprecation warning, but probably should.

   #!/python
   from twisted.python.text import stringyString
   

   I think twisted.protocols.telnet is a better example. Note, this will involve suppressing the warning from the toplevel import, and perhaps more work to test the deprecation. (This probably also means the tests don't need to be touched)
2. There are some twistedchecker errors.
3. Is there a good reason to port the module? (I haven't looked at these changes, except to note that we don't have full test coverage)
4. There don't appear to be tests for the copied functions. (And the one in lore should probably be private.)
5. The core topfile should be a .removal.
6. The lack of test coverage for almost all of the changes makes me sad. (Most of the changed code isn't exercised, let alone tested)

[Thijs Triemstra]

(In [38002]) address review comments, refs #6341

[Thijs Triemstra]

(In [38041]) address review comments, refs #6341

[Thijs Triemstra]

Replying to tom.prince:

2. There are some twistedchecker errors.

Fixed.

3. Is there a good reason to port the module? (I haven't looked at these changes, except to note that we don't have full test coverage)

I guess not, removed it from the ported list.

4. There don't appear to be tests for the copied functions. (And the one in lore should probably be private.)

The new ones in t.lore.tree are covered according to coverage. Copied the tests for imap4._strFile.

5. The core topfile should be a .removal.

Fixed.

[Tom Prince]

Is this meant to be in review? In any case, here are a couple of comments:

1. There appears to be some debugging changes or something in twisted/conch/insults/window.py (a use of t.p.text).
2. The code in lore gets run, but it doesn't appear that its behavior isn't really tested. In any case, it's behavior is straightforward enough that it should be easy to write tests for it.

[Thijs Triemstra]

No, not yet (partly because of point 1 of your review), but thanks anyway.

[Richard Wall]

See also #1601 where one use of text.wordWrap is being removed.

[Thijs Triemstra]

(In [38391]) Branching to 'py3-text-6341-2'

[Thijs Triemstra]

1. twisted/lore/latex.py:
   1. Added test coverage for changed lines
2. twisted/lore/slides.py:
   1. Added test coverage for changed lines
3. twisted/lore/tree.py:
   1. Added test coverage for new methods (but _removeLeadingBlanks doesn't make sense to me, seems buggy?)
4. twisted/lore/texi.py:
   1. Added test coverage for changed lines in new test_texi case
5. twisted/mail/imap4.py:
   1. Added test coverage for changed lines
6. twisted/python/usage.py:
   1. Changes were already covered (also see #1601)
7. twisted/words/protocols/irc.py:
   1. Added test coverage based on some of Wolf's code from #5329
8. twisted/conch/insults/window.py:
   1. Added test coverage
9. twisted.python.text:
   1. Changed the deprecation to mimic the telnet deprecation (added in r14536) and importing it in different ways displays the DeprecationWarning, only I'm not sure how to test/suppress this, any pointers are welcome.

Forced a new ​build, changes here: [log:branches/py3-text-6341-2].

[therve]

OK, it may be because I'm tired, or because the review queue is very long, but I think this ticket is improper. A 1500 lines diff is not a reasonable thing to do, in particular in this case. Please open at least 4 other tickets to handle changes per subprojects. Sorry for the delay in responding on this.

[Thijs Triemstra]

Replying to therve:

OK, it may be because I'm tired, or because the review queue is very long, but I think this ticket is improper. A 1500 lines diff is not a reasonable thing to do, in particular in this case. Please open at least 4 other tickets to handle changes per subprojects. Sorry for the delay in responding on this.

Thanks for the review. I opened 4 additional tickets (see updated ticket description for ticket nrs) for the subprojects. I'll leave this ticket open as the main ticket that will actually deprecate the module and replace it's usage in core, once these other tickets have been resolved.

[Thijs Triemstra]

(In [39954]) Merge replace-text-6546: Replace usage of twisted.python.text in twisted.lore.

Author: thijs Reviewer: rwall, tom.prince Fixes: #6546 Refs: #6341

Removes the usage of twisted.python.text in twisted.lore because it's being deprecated. Adds basic tests for lore.texi as well.

[Jonathan Ballet]

I made a port to Python 3 of twisted.python.usage which used twisted.python.text.wordWrap in #7038 and thus I replaced the usage of wordWrap by textwrap.wrap.