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 to provide option for making trailing whitespace visible #67855

Closed
rhettinger opened this issue Mar 14, 2015 · 11 comments
Closed

IDLE to provide option for making trailing whitespace visible #67855

rhettinger opened this issue Mar 14, 2015 · 11 comments
Assignees
Labels
3.7 (EOL) end of life 3.8 only security fixes topic-IDLE type-feature A feature request or enhancement

Comments

@rhettinger
Copy link
Contributor

BPO 23667
Nosy @rhettinger, @terryjreedy, @taleinat, @mlouielu, @csabella
PRs
  • bpo-23667: IDLE: Add trailing space highlight #1602
  • bpo-23667: IDLE: Add trim trailing whitespace and blankline on save extension #1603
  • bpo-23667: IDLE: Add trailing space highlight #1642
  • bpo-23667: IDLE: Add trailing space highlight  #1643
  • bpo-23667: IDLE: Add trim extension to trim trailing whitespace and blankline on save #1644
  • Superseder
  • bpo-33046: IDLE option to strip trailing whitespace automatically on save
  • 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 = <Date 2019-11-18.04:51:36.930>
    created_at = <Date 2015-03-14.23:43:04.256>
    labels = ['3.8', 'expert-IDLE', 'type-feature', '3.7']
    title = 'IDLE to provide option for making trailing whitespace visible'
    updated_at = <Date 2019-11-18.04:51:36.924>
    user = 'https://github.com/rhettinger'

    bugs.python.org fields:

    activity = <Date 2019-11-18.04:51:36.924>
    actor = 'terry.reedy'
    assignee = 'terry.reedy'
    closed = True
    closed_date = <Date 2019-11-18.04:51:36.930>
    closer = 'terry.reedy'
    components = ['IDLE']
    creation = <Date 2015-03-14.23:43:04.256>
    creator = 'rhettinger'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 23667
    keywords = []
    message_count = 11.0
    messages = ['238110', '238741', '293757', '293910', '293911', '293984', '294005', '294043', '344109', '356828', '356840']
    nosy_count = 5.0
    nosy_names = ['rhettinger', 'terry.reedy', 'taleinat', 'louielu', 'cheryl.sabella']
    pr_nums = ['1602', '1603', '1642', '1643', '1644']
    priority = 'normal'
    resolution = 'rejected'
    stage = 'resolved'
    status = 'closed'
    superseder = '33046'
    type = 'enhancement'
    url = 'https://bugs.python.org/issue23667'
    versions = ['Python 3.7', 'Python 3.8']

    @rhettinger
    Copy link
    Contributor Author

    We have an option to clear trailing whitespace (Format / Strip trailing whitespace) but it isn't always clear when to use it (because the trailing whitespace is invisible).

    One thing the can be done is to colorize trailing white space to make it more visible (this idea is inspired by a similar feature in mercurial's colorized diffs).

    Another option is put a hook on File / Save to scan for trailing whitespace and offer to remove it (this idea is inspired by the whitespace commit hook used in Python's own source code management).

    @rhettinger rhettinger added topic-IDLE type-feature A feature request or enhancement labels Mar 14, 2015
    @terryjreedy
    Copy link
    Member

    I agree that this area needs improvement. 'Strip trailing whitespace' should strip trailing blank lines at the end of a file, as well as trailing blank spaces at the end of each line. Both are required to commit to our repository.

    I would like to have a configuration option to always strip trailing blanks, at least for .py files.

    I believe the cursor moving from one line to another is an event that is or could be caugth and acted on. If so, trailing spaces could be removed immediately from the line left, and Save would only have to check the current line and the end of the file. I would rather remove blanks than mark them for later removal.

    @mlouielu
    Copy link
    Mannequin

    mlouielu mannequin commented May 16, 2017

    PR 1602 add trailing whitespace highlight, which using color from error.

    PR 1603 add TrimExtension to provide auto trim while saving the file.

    @terryjreedy terryjreedy added the 3.7 (EOL) end of life label May 16, 2017
    @terryjreedy
    Copy link
    Member

    Louie, when you close and open PRs, please give at least a brief explanation. In particular, how is the new one different?

    @terryjreedy terryjreedy self-assigned this May 18, 2017
    @mlouielu
    Copy link
    Mannequin

    mlouielu mannequin commented May 18, 2017

    terry, these close isn't changing anything. But I'm migrating my GitHub account from old one to new one, thus will close the PR and reopen another one, sorry for annoying mailing.

    @csabella
    Copy link
    Contributor

    Terry,

    Should IDLE library modules get docstrings or is that something you'd rather not have? Putting the question here because ColorDelegator in colorizer.py doesn't have any docstrings. I wanted to read some of the IDLE code and I can try to add missing docstrings if it's something that should be done.

    Thanks!

    @terryjreedy
    Copy link
    Member

    Louie: It appears that old PRs might merge as they are but any change, including an update merge, requires that you close and re-open. When you re-open, please add "was prnnnn". It would be easier for me if you redid all the IDLE PRs in a batch or two.

    Cheryl: Yes. Idlelib callables need docstrings, especially because they are not otherwise documented. Based on our work on bdb, I was hoping you would work on IDLE. I added you to bpo-30290

    Some of the colorizer functions are hard to usefully characterize. This is parly because everything is done by side-effect and most functions have multiple side-effects. There is a connection between the lack of docstrings and the vestigial nature of test_colorizer.

    @csabella
    Copy link
    Contributor

    Thanks, Terry! I'll take a look at 30290. No need to start with colorizer; it was in this ticket so I referenced it more for convenience than anything.

    @terryjreedy
    Copy link
    Member

    Raymond, which definition of 'whitespace' do you intent?

    Git gui also marks trailing spaces in frozen diffs. This is much harder to do while editing as text can be read, keyed, and pasted. Most spaces and blank lines are only temporarily trailing, before something more is added. I believe I would dislike the idea of temporarily marking them, unless it were very subtle. They really only need to be stripped just before saving. So I think we should make that easy and not too intrusive.

    1. Ask if there is trailing whitespace. For instance,
      if (text.find('\s\n') != -1) or <text ends with '\n\n'>:
      <Query 'strip?', with 'Yes' the default for .py files>

    2. Add a setting to strip on save, default yes.

    3. always strip trailing whitespace from .py files when saving.
      Perhaps flash 'trailing whitespace stripped' to status bar.

    The danger with any of these is stripping of wanted trailing whitespace in tripple-quoted string literals. Such whitespace is rare, but I don't know how rare.

    Alternate ways of making such are better in making the blanks permanently visible in any display, not just in IDLE. The problem is teaching this.

    >>> s = (
      'This is justified text\n'
      'with blank spaces.    \n'
      'It is is followed by  \n'
      'a trailing blank line.\n'
      '\n')
    >>> s
    'This is justified text\nwith blank spaces.    \nIt is is followed by  \na trailing blank line.\n\n'

    while an error marking would tell users to do it manually.
    ---

    I closed PR 1644 because implementing features as extensions is obsolete and because the trim function duplicates the existing function (once it is fixed to include blank lines).

    I am dubious about PR 1643. Tagging trailing whitespace as an error would suggest that the user should delete it. With strip on save, this would be noise.

    @terryjreedy terryjreedy added the 3.8 only security fixes label May 31, 2019
    @taleinat
    Copy link
    Contributor

    See also bpo-33046 specifically suggesting auto-removal of trailing whitespace on saving a file, as per Raymond's second suggestion here. It has a PR which seems about ready to go.

    I see that as the simplest approach to this issue, both in terms of implementation and in terms of UI/UX.

    Could we mark that issue as superseding this one?

    @terryjreedy
    Copy link
    Member

    Given the issues I raised and Raymond's rejection of this, his first suggested option, on the PR, in favor of his second suggested option, bpo-33046, I agree, at least for now.

    The use case for whitespace tagging would be if someone wants to selectively delete trailing whitespace. However, python/cpython rejects any trailing whitespace for all of .py, .c, and .rst files. So this must be exceedingly rare and is not something that IDLE must facilitate.

    @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.7 (EOL) end of life 3.8 only security fixes topic-IDLE type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants