classification
Title: Idle: enhance FormatParagraph
Type: enhancement Stage: test needed
Components: IDLE Versions: Python 3.7, Python 3.6
process
Status: open Resolution:
Dependencies: 18226 Superseder:
Assigned To: terry.reedy Nosy List: JayKrish, Todd.Rovito, philwebster, terry.reedy
Priority: normal Keywords: patch

Created on 2013-07-29 02:39 by terry.reedy, last changed 2017-06-19 23:52 by terry.reedy.

Files
File name Uploaded Description Edit
18583enhancefp1.patch philwebster, 2013-08-12 23:20 review
Messages (5)
msg193848 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-07-29 02:39
Writing tests for FormatParagraph.py #18226 revealed that it could use some improvements.

At minimum, some for format_paragraph should probably be replaced with textwrap.wrap (or the class) in order to handles double spacing after sentences. 

Find_paragraph should find the boundaries of triple-quoted strings.

The current end-of-text behavior of find_paragraph should perhaps be changed.
msg194768 - (view) Author: Phil Webster (philwebster) * Date: 2013-08-09 20:23
In the case of no selection would it make sense to only format if the cursor is in a string/comment? If not in a string or comment, the single line that the cursor is in could be formatted with a line ending backslash or not at all. Otherwise Format Paragraph will probably just be mangling code.
msg195036 - (view) Author: Phil Webster (philwebster) * Date: 2013-08-12 23:20
I've attached a patch that attempts to address the issues above. For the tests, I made some changes because the cursor doesn't end in exactly the same spot with line endings.

Here's a quick summary of the changes:
1. Removed format_paragraph and format_comment methods because all text is treated the same and formatted in the event method. (The formatting code could be pulled out of the format_event method if needed.)

2. Added find_string_indices method to see if cursor is in string using the text widget's string tag. If the cursor is in a string, only the string lines are formatted. Before, if code immediately preceded or followed the string at the same indentation level, it was treated as part of the same paragraph.

3. If the text to be formatted ends with a triple quoted string on its own line (such as a docstring), the formatting code leaves the line alone.

4. If selection contains multiple paragraphs (separated by blank lines), it can format each one. Previously, only the first paragraph was formatted.

Once I have some feedback, I can thoroughly check all of the tests and make sure that we're getting the expected behavior.
msg195094 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-08-13 18:54
Based on working with the #18226 patch, I now think that rewrapping partial lines is a bug. Although I removed some problematical tests, I think there is still one that verifies buggy behavior. The outline of steps (which necessarily omits some details) would then be:

1. Extract a list of complete lines (without \n) from the text widget. The only purpose of using selections rather than the cursor would be to get partial paragraphs or multiple paragraphs. (I had thought of deleting that idea, but if you have made it work, I will look at it.)

There are still details to work out. If the cursor is on a blank line, should we really search forward? Normally, sel.first would be moved back to the beginning of the line [you apparently have already done this] and sel.last to the end. But if a block selection ends at the beginning of a line, sel.last should not be moved.

2. Delete the common prefix from each line, including '#' for comment blocks. Do this and the following without the current repeating joining of and splitting into lines. It is all wasted motion.

3. Reformat into a new list of lines, now with \n.

4. Add back the common prefix (still a list of lines with \n).

5. Insert the lines (which already have \n) into the text widget, one at a time.

   One of the details I left out is consistently handling of \n for the last line, so that deletion from the text widget and inserting into the widget match. One of the 'features' of the current code is that it will handle paragraphs with a different indent on the first line. I am not sure it is needed, as it does not apply to comment and string blocks.

Steps 2,3,4 do not involve the text widget. I think that they should be performed in one or more non-widget functions so they can be separately tested. I would feel this way even if mock_tk.Text were complete enough to substitute for tkinter.Text in the method test. 

I have not looked at your patch much because I want to review and possibly edit and commit some of the other submitted test patches before working more on this.
msg195118 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-08-14 02:12
Note to myself. The current test suite has one test commented out because it worked with EditorWindow but not the mock. It is possible that it is a mark-gravity issue. (I ran into this with the mock for IdleHistory.).
History
Date User Action Args
2017-06-19 23:52:49terry.reedysetversions: + Python 3.7
2017-06-19 23:51:08terry.reedysetcomponents: + IDLE
versions: + Python 3.6, - Python 2.7, Python 3.3, Python 3.4
2013-08-14 02:12:01terry.reedysetmessages: + msg195118
2013-08-13 18:54:49terry.reedysetmessages: + msg195094
2013-08-13 02:10:05JayKrishsetnosy: + JayKrish
2013-08-12 23:20:51philwebstersetfiles: + 18583enhancefp1.patch
keywords: + patch
messages: + msg195036
2013-08-10 21:24:34terry.reedylinkissue18226 superseder
2013-08-09 20:23:24philwebstersetmessages: + msg194768
2013-08-08 05:12:14terry.reedysetdependencies: + IDLE Unit test for FormatParagrah.py
2013-07-30 02:45:48Todd.Rovitosetnosy: + Todd.Rovito
2013-07-30 01:30:56philwebstersetnosy: + philwebster
2013-07-29 02:39:37terry.reedycreate