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

Idle: enhance FormatParagraph #62783

Open
terryjreedy opened this issue Jul 29, 2013 · 5 comments
Open

Idle: enhance FormatParagraph #62783

terryjreedy opened this issue Jul 29, 2013 · 5 comments
Assignees
Labels
3.10 only security fixes topic-IDLE type-feature A feature request or enhancement

Comments

@terryjreedy
Copy link
Member

BPO 18583
Nosy @terryjreedy, @rovitotv
Dependencies
  • bpo-18226: IDLE Unit test for FormatParagrah.py
  • Files
  • 18583enhancefp1.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/terryjreedy'
    closed_at = None
    created_at = <Date 2013-07-29.02:39:37.358>
    labels = ['expert-IDLE', 'type-feature', '3.10']
    title = 'Idle: enhance FormatParagraph'
    updated_at = <Date 2020-06-07.20:44:27.271>
    user = 'https://github.com/terryjreedy'

    bugs.python.org fields:

    activity = <Date 2020-06-07.20:44:27.271>
    actor = 'terry.reedy'
    assignee = 'terry.reedy'
    closed = False
    closed_date = None
    closer = None
    components = ['IDLE']
    creation = <Date 2013-07-29.02:39:37.358>
    creator = 'terry.reedy'
    dependencies = ['18226']
    files = ['31266']
    hgrepos = []
    issue_num = 18583
    keywords = ['patch']
    message_count = 5.0
    messages = ['193848', '194768', '195036', '195094', '195118']
    nosy_count = 4.0
    nosy_names = ['terry.reedy', 'Todd.Rovito', 'JayKrish', 'philwebster']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'test needed'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue18583'
    versions = ['Python 3.10']

    @terryjreedy
    Copy link
    Member Author

    Writing tests for FormatParagraph.py bpo-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.

    @terryjreedy terryjreedy self-assigned this Jul 29, 2013
    @terryjreedy terryjreedy added the type-feature A feature request or enhancement label Jul 29, 2013
    @philwebster
    Copy link
    Mannequin

    philwebster mannequin commented Aug 9, 2013

    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.

    @philwebster
    Copy link
    Mannequin

    philwebster mannequin commented Aug 12, 2013

    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.

    @terryjreedy
    Copy link
    Member Author

    Based on working with the bpo-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.

    1. 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.

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

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

    4. 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.

    @terryjreedy
    Copy link
    Member Author

    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.).

    @terryjreedy terryjreedy added 3.10 only security fixes and removed 3.7 (EOL) end of life labels Jun 7, 2020
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes topic-IDLE type-feature A feature request or enhancement
    Projects
    Status: No status
    Development

    No branches or pull requests

    1 participant