Skip to content

bpo-43894: Editor reformat #25485

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

Closed
wants to merge 5 commits into from

Conversation

E-Paine
Copy link
Contributor

@E-Paine E-Paine commented Apr 20, 2021

@E-Paine
Copy link
Contributor Author

E-Paine commented Apr 20, 2021

I presume this doesn't need a news entry, but I am happy to added one if necessary.

@E-Paine
Copy link
Contributor Author

E-Paine commented Apr 23, 2021

Following the changes proposed in #25479, I would like a little more time on this PR before review (no point commenting on exactly the same things).

@E-Paine E-Paine force-pushed the idlelib-editor-colorizer branch from c3ab35d to b0eb61e Compare April 23, 2021 14:08
@terryjreedy
Copy link
Member

OK, say when done.

@E-Paine
Copy link
Contributor Author

E-Paine commented Apr 23, 2021

@terryjreedy I think this is ready now. I would appreciate your particular scrutiny of where I have added an explicit check for is (not) None, as I had to revert a few because they caused issues during testing (despite me thinking I checked they were safe...).

@terryjreedy
Copy link
Member

Can you describe the 'issues'? Could you have uncovered a bug? Or are you really convinced that your initial thought was wrong?

@E-Paine
Copy link
Contributor Author

E-Paine commented Apr 23, 2021

Sorry, just things like passing a blank string to tkinter and so getting an error. This was a regression with my change (a blank string would be treated as false but my is not None would evaluate to true) and has been reverted.

@E-Paine
Copy link
Contributor Author

E-Paine commented Apr 28, 2021

Given that it is highly unlikely this is going to be merged (at least in the current form), I am closing this PR. I will leave the issue on bpo open for now.

@E-Paine E-Paine closed this Apr 28, 2021
@terryjreedy
Copy link
Member

Yes, leave issue open. My quick review is that I eventually want most but not all of the changes. (In particular, "Return xyz." is a legal docstring.) Some of the changes I would like one type at a time. There is an existing issue with more tests that is probably about ready to merge once I review it. But working on shell indents and sidebar now. So wait until I ask for something in particular.

@E-Paine E-Paine mannequin mentioned this pull request Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants