Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

gh-77227: An IDLE option to strip trailing whitespace on save#17201

Open
ZackerySpytz wants to merge 2 commits into
python:mainpython/cpython:mainfrom
ZackerySpytz:bpo-33046-strip-trailing-whitespace-optionZackerySpytz/cpython:bpo-33046-strip-trailing-whitespace-optionCopy head branch name to clipboard
Open

gh-77227: An IDLE option to strip trailing whitespace on save#17201
ZackerySpytz wants to merge 2 commits into
python:mainpython/cpython:mainfrom
ZackerySpytz:bpo-33046-strip-trailing-whitespace-optionZackerySpytz/cpython:bpo-33046-strip-trailing-whitespace-optionCopy head branch name to clipboard

Conversation

@ZackerySpytz

@ZackerySpytz ZackerySpytz commented Nov 17, 2019

Copy link
Copy Markdown
Contributor

@ZackerySpytz

Copy link
Copy Markdown
Contributor Author

There are calls to undo_block_start() and undo_block_stop() in do_rstrip(), which means that saving a file with the option enabled will add an undo command. I don't think this is preferable. I guess a new var could be added to the Rstrip class in order to prevent the undo_block_*() calls in this case.

@terryjreedy

Copy link
Copy Markdown
Member

I am thinking about the undo issue to make sure I agree.

@taleinat

taleinat commented Nov 17, 2019

Copy link
Copy Markdown
Contributor

Having an item in the undo history is needed, otherwise undoing changes before the whitespace stripping could potentially be broken. I don't really see a way to avoid it.

@taleinat taleinat left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@terryjreedy terryjreedy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some of the changes you can do now, some need more input from me.

font-bold= 0
encoding= none
line-numbers-default= 0
strip-trailing-whitespace-on-save= 1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

rstrip-on-save might be sufficient.

Except for line-numbers, the other existing items are actually for the base class used for editor, shell, and output windows. This has to be for editor windows only, at least at present. PyShell subclasses code.Interactive Interpreter, which already does some stripping before compiling. And user code output should not be touched. This different between items should somehow be more obvious on the configdialog tab. I am thinking about what to do.

If someone want to disable this, doing so for particular windows makes more sense than globally. Could we get away with having only a window option, or are there really people who would never want

Comment thread Lib/idlelib/iomenu.py
return "break"

def writefile(self, filename):
strip = idleConf.GetOption(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If rstrip_save is a per-window property, this will not work.

Comment thread Lib/idlelib/iomenu.py
'main', 'EditorWindow',
'strip-trailing-whitespace-on-save', type='bool')
if strip:
self.editwin.Rstrip(self.editwin).do_rstrip()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There is already a per-window event handler which I believe could be reused.

@bedevere-bot

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@terryjreedy

Copy link
Copy Markdown
Member

On undo, I will go with what Tal says until an actual problem becomes evident.

@csabella

Copy link
Copy Markdown
Contributor

@ZackerySpytz, please address the code review. Thanks!

@taleinat

Copy link
Copy Markdown
Contributor

Ping, @ZackerySpytz?

@hauntsaninja hauntsaninja changed the title bpo-33046: An IDLE option to strip trailing whitespace on save gh-77227: An IDLE option to strip trailing whitespace on save Jan 7, 2023
@github-actions

github-actions Bot commented Apr 7, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting changes stale Stale PR or inactive for long period of time.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.