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

Add auto-indent for Python #15159

Closed
wants to merge 2 commits into from

Conversation

alankilborn
Copy link
Contributor

Fix #15122

@alankilborn
Copy link
Contributor Author

@BlueBatSamurai
If you want to test this implementation of your feature request, that would be great.

@chcg chcg added the feature Feature requests and Feature commits label May 20, 2024
@donho donho self-assigned this May 21, 2024
@donho
Copy link
Member

donho commented May 21, 2024

Can any one here knows Python test auto-indent of this PR please?

@mpheath
Copy link
Contributor

mpheath commented May 21, 2024

Test by inserting the caret to the EOL and pressing Enter

:
": #"
""":
:#"""
else:
elsse:

The else: should indent the next line and the others should not indent the next line.

Currently, the Python Indent plugin installable from Plugin Admin is the better choice IMO as it determines conditionally to indent or not to indent. Indenting almost any line with a colon at EOL as the PR does can be disruptive if the indenting is incorrect. When both plugin and PR indenting are enabled, the plugin fixes the actions of the PR indenting and then needs more undo actions so it is worse for undo to have both enabled. Disabling Auto-indent checkbox prevents the extra undo behaviour though affects all tabs that may have other languages where Auto-indent is preferred.

Builtin auto indenting for Python seems like a nice idea and the code looks tidy, I suggest to improve the regex to lessen indenting that should not occur:

"^\\h*(?:async|case|class|def|elif|else|except|finally|for|if|match|try|while|with)\\b.*?:\\h*(?:#|$)"

or perhaps make async with the following keyword more robust:

"^\\h*(?:async\\h+def|async\\h+for|async\\h+with|case|class|def|elif|else|except|finally|for|if|match|try|while|with)\\b.*?:\\h*(?:#|$)"

The Python Indent plugin checks for these keywords. plugin code

@alankilborn
Copy link
Contributor Author

@mpheath

Thanks for killing my PR. :-)

I was unaware that there was a plugin that did this functionality; I have long used a script to do it, but when the 15122 issue came along I thought it made sense to code it natively.

The idea behind the PR code was to keep it simple: If a line "ends" with a colon (:), indent the next line one more level. Although...I did get a little fancy with it by allowing whitespace and/or a comment to be after the colon (because I myself very often put a comment on an "if :" line).

C-like auto-indent behavior for other languages in N++ is far from perfect, and again I was trying to keep Python indenting simple and not "perfect".


Indenting almost any line with a colon at EOL ... can be disruptive if the indenting is incorrect.

I don't know what this means. IMO indenting after any line with a : as the last real character is fine, so can you provide a counter example(s)?


When both plugin and PR indenting are enabled, the plugin fixes the actions of the PR indenting and then needs more undo actions so it is worse for undo to have both enabled.

Why even try it with plugin and this PR together? Makes no sense.


I suggest to improve the regex

I'd prefer not, mainly because I don't think it is necessary.
But I would, to get a code change accepted.
And also to prevent future nay-sayers saying that the plugin works better than the native code.


Builtin auto indenting for Python seems like a nice idea and the code looks tidy

Well...something positive! :-)
Thanks for your comments and opinions; if they kill the PR, which I think is extremely likely...so be it.

@donho
Copy link
Member

donho commented May 21, 2024

@mpheath
Thank you for testing and the provided information!

@alankilborn

Thanks for killing my PR. :-)

Not necessarily.
It's better if the feature is complete. But if not, the idea is to provide a rudimentary functionality:
The new user can benefit immediately from the builtin basic Python auto-indent, without knowing the existence of the plugin.
For the experienced users who are not satisfied with the builtin basic feature, they can always install the plugin to override the builtin functionality.

@alankilborn
Copy link
Contributor Author

@donho

OK, I like your "basic functionality" statement.

...So what should be done next to move this PR forward?

@donho
Copy link
Member

donho commented May 21, 2024

...So what should be done next to move this PR forward?

I will test it as well, to make sure it's useful for the new users, then do the code review. :)

@BlueBatSamurai
Copy link

BlueBatSamurai commented May 21, 2024

@BlueBatSamurai If you want to test this implementation of your feature request, that would be great.

Sorry but I’m new on GitHub…
Could anyone tell me how to use this in Notepad++?
I’ve seen the linked code, but what do I have to do with it?

@alankilborn
Copy link
Contributor Author

@BlueBatSamurai
Get new notepad++.exe from HERE.
Rename to something like notepad++_PR15159.exe.
Copy into the folder where your existing notepad++.exe is.
Close Notepad++ if you happen to have it running.
Run notepad++_PR15159.exe.

@BlueBatSamurai
Copy link

@alankilborn
Thanks!

@BlueBatSamurai
Copy link

BlueBatSamurai commented May 22, 2024

I've dowloaded and renamed the new Notepad++.exe.

And it works! Even if there is a comment behind the colon before you press Enter.
Well done!

(And yes, it doesn't check whether there is a valid keyword before the colon... Something like qwert: + Enter or just a single : causes indentation too. But the user should know that for themselves.)

Copy link
Member

@donho donho left a comment

Choose a reason for hiding this comment

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

Please follow the suggestion:


_pEditView->execute(SCI_SETSEARCHFLAGS, SCFIND_REGEXP | SCFIND_POSIX);
_pEditView->execute(SCI_SETTARGETRANGE, _pEditView->execute(SCI_POSITIONFROMLINE, prevLine),
_pEditView->execute(SCI_GETLINEENDPOSITION, prevLine));
Copy link
Member

Choose a reason for hiding this comment

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

Improve readability:

auto startPos = _pEditView->execute(SCI_POSITIONFROMLINE, prevLine);
auto endPos = _pEditView->execute(SCI_GETLINEENDPOSITION, prevLine);
_pEditView->execute(SCI_SETTARGETRANGE, startPos, endPos);

PowerEditor/src/Notepad_plus.cpp Show resolved Hide resolved
@donho donho added the accepted label May 25, 2024
@donho donho closed this in b3b90a5 May 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted feature Feature requests and Feature commits
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature request] Auto-indentation in Python
5 participants