Thanks for giving this a whirl, @aeros167!

From an initial review of the GUI, it looks like the font of the characters in the sidebar aren't scaling dynamically when the font is adjusted in the shell

Good catch! Fixed.

1. I do not intend to rush this into b4, but do hope to get it into 3.7.5rc1 and 3.8.0.rc1.

2. Kyle, thank you for reviewing. On of the things I worked on todays was Tal's other recent patch, a bugfix for a regression, which I wanted and which will be in b4.

3. About history traversal: I have not looked at the mechanism in years. I believe I was thinking that history could traverse '>>>' labels in the sidebar. Not all stdin text is code, as the same tag is used for input('prompt') responses, which is why the latter are colorized as code. (Unsolved bug.) I know I intend to use the labels for a future 'code input' only save. Does tk coalesce successive blocks tagged the same? Does an untagged '\n' separate such? (Looking back, I see you are tagging '\n' as an invisible prompt. Clever idea.

3. Does tk coalesce successive blocks tagged the same?

Yes.

Does an untagged '\n' separate such?

Yes, but final '\n' chars of input blocks are currently also tagged "stdin", and changing that would be fiddly.

(Looking back, I see you are tagging '\n' as an invisible prompt. Clever idea.

Indeed. Note that these are tagged both "console" and "stdin".

Thanks for giving this a whirl, @aeros167!

Kyle, thank you for reviewing.

No problem. Feel free to @mention me or add me to the reviewers for any IDLE-related testing. GUI design isn't my main area of experience, but as a regular user of it I'm glad to help out with testing new changes. (:

Does an untagged '\n' separate such?

Yes, but final '\n' chars of input blocks are currently also tagged "stdin", and changing that would be fiddly.

On second thought, removing the "stdin" tag from those newlines might be relatively simple.

I seem to have things working rather well as they are right now, though. I can take a look at making such a change if you like, @terryjreedy, if we decide to move forward with this and everything else looks okay.

I like the look of this and it seems to run smoothly.

1. The most essential improvement is to use the configured space indent both for tab and smart indents. This should partly be a matter of removing stuff that overrides the editor settings. I am not sure if smart indent still needs to know when code is in Shell. Cheryl and I worked on it in the last year or so to fix bugs.

2. Much less important: fix pre-existing glitch with multiline statements. It looks worse now.
   a) Backspace to remove smart indent before hitting return.

>>> if 1:
...         1
... 
    1
>>> 

I believe I would prefer removal of the final ... before the statement ending the blank line. However, it presence matches the console, though there is no option there. Not essential. (Note that I had to replace \t with spaces to look correct.

b) Hit return to end statement without backspacing.

>>> if 2:
...         2
... 
...         # End of still present smart indent, which was originally a tab.
    2
>>> 

A return is inserted both before and after the smart indent. Instead, I think the smart indent should be removed, as if never inserted. In the Windows console, at least, return after a (manual) indent does not terminate a statement. I believe that this is why a newline is also inserted before the indent, so people do not have to hit return a third time to end the statement.

This console behavior allows one to put visually blank lines in multiline statements, but I doubt people do this much. I have never seen a complaint not being able to do this in IDLE, and I think it a bit confusing.

>>> if 4:
...   5
...  
...   if 5: 7
...
5
7
>>> if 5:
...   8
...
... if 9: 10
  File "<stdin>", line 4
    if 9: 10
    ^
SyntaxError: invalid syntax

3. Nice sometime: Removed the unused prompt before restart lines, and any statement marker added. Then remaining prompts other than the last indicate entered and executed statements.

    4
>>> 
    ================ RESTART: Shell ======================
>>> 

We can bikeshed over whether to leave a blank line after experience both ways.

4. I think there are some glitches with coloring (backgrounds) but I need to make a theme with stronger diffrences to be sure of what is going on.

1b (important). Add a way to save Shell so that statements (or rather non-statements) are clearly marked. Being able to get executable saves is another reason for moving prompts away from code.

At this point, I need to read the code so I have some idea of the difficulty of various possible changes.

@terryjreedy:

This console behavior allows one to put visually blank lines in multiline statements, but I doubt people do this much. I have never seen a complaint not being able to do this in IDLE, and I think it a bit confusing.

I would agree. Whitespace and formatting concerns are generally not particularly important when using the shell.

@terryjreedy, perhaps I'm missing something, but most of the points you've just raised (1, 2, 3, and 1b) seem like further enhancements you'd like to make that could benefit from this change. As they are not directly related to this PR, perhaps they would be better discussed elsewhere?

Regarding the background glitches (4), I'll try to take a look.

2 & 3 were recorded here as I reviewed, but should be separate patches (and recorded on the main shell revamp issue). Sidebars clicks can also be a future issue.

As to 1, we are both missing something. You apparent think of this as a standalone patch, whereas I was only thinking of it as part of getting rid of tab indents and matching editor space indents. That said, I really like having a proper even margin for multiline statements, even with the tabs still there.

My concern is that I expect some people will object to just this change. Others might like it because the addition of '... ' secondary prompts makes Shell look more like the standard REPL. Others who have never used the latter will not know.

As for 1b, Shell saves, the prompt removal is both a minus as far as finding code chunks but a plus as far as running code chunks. I would prefer to include something that is a complete plus.

For 3.8.0rc1, scheduled Monday 9-30, this should be a moot point as both this and a couple of followups should be merged. For 3.7.5rc1, scheduled Monday 09-16, I am not so sure. Should this be released by itself, or should we consider not backporting to 3.7 until after.

@aeros167 Do you use IDLE regularly? Would you want to see this in 3.7.5 even any followup patches?

In the meanwhile, I will review this as feature-complete.

@terryjreedy:

My concern is that I expect some people will object to just this change. Others might like it because the addition of '... ' secondary prompts makes Shell look more like the standard REPL. Others who have never used the latter will not know.

Perhaps to get an idea of the popularity of the change, we could open a poll on https://discuss.python.org/c/users. The detailed feedback is more valuable, but the polls tend to draw in more participants. Probably because they require a very minimal time investment.

However, I would think that most people who have used the IDLE Shell have also used the standard REPL at some point. This may be anecdotal, but I've not interacted with anyone who has used the IDLE Shell and not the REPL. From my experience, it tends to be the other way around.

My interactions with other Python users in person has been primarily through college, which may slightly skew the demographics. But if anything, I would think that newer users of Python would be the ones more likely to have exclusively used the IDLE shell since the REPL has been around for longer.

@aeros167 Do you use IDLE regularly? Would you want to see this in 3.7.5 even any followup patches?

I would personally be interested in seeing this change in 3.7.5. I typically tend to alternate between using the REPL for the most simplistic usages of Python (checking the length of a line, scientific calculator, etc), IDLE for more involved tasks or scripts, and the vscode python extension for projects. I always thought it was a little bit odd that the IDLE shell didn't have the ellipsis prompts, but never thought to open an issue for it.

If I were to personally prioritize the followup patches, I would say the second one you mentioned would be the most significant to me:

Much less important: fix pre-existing glitch with multiline statements. It looks worse now.

This is just my personal preference though. Even if they're less impactful at the end of the day, the visual bugs (even minor ones) have a tendency to stand out to me. Especially for visual bugs that would occur more commonly, such as this one.

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@aeros167 Thank you for the response. If you like the missing dots back, I suspect others will too.

I don't want opinions based on imagination. Best would be that a person use the new version for at least 15 minutes, with a variety of statements. Minimum would be looking at a picture of a session with at least a few statements entered. If you feel like it, it might be helpful if you could upload a screenshot of duplicate before (installed) and after (patched repository) sessions side by side. I may ask Ned Deily, the 3.7 release manager, for an opinion, or permission to add a patch after the release of the candidate.

@taleinat "Removing prompts from the text widget made knowing where each statement started impossible, but the sidebar needs to know this. This PR adds the "console" tag to the newline before each statement to overcome this." A couple of question based on not having read the shell
sidebar (SSB) code yet.

Q1: Does SSB keep track of where text is (apparently) frozen, so it only rescans new stuff, and not start at the beginning each time?
Q2: If the console tag were removed from a newline and SSB were told to back up to that untagged newline, would it remove the '>>>' it previously inserted? This will also come up if text, such as async output or warnings, needs to be inserted before an entry line.

About history recall: a glitch new to me of skipping 1,2 char name expressions remains after the patch.

>>> a=1
>>> b
Traceback (most recent call last):
  File "<pyshell#14>", line 1, in <module>
    b
NameError: name 'b' is not defined
>>> a+b
Traceback (most recent call last):
  File "<pyshell#15>", line 1, in <module>
    a+b
NameError: name 'b' is not defined
>>> bbb
Traceback (most recent call last):
  File "<pyshell#16>", line 1, in <module>
    bbb
NameError: name 'bbb' is not defined
>>> bb
Traceback (most recent call last):
  File "<pyshell#17>", line 1, in <module>
    bb
NameError: name 'bb' is not defined
>>>

Hit history-prev (Alt-P for me) several times and 'bb' is skipped, 'bbb' is shown, 'a+b' is shown, b is skipped, and 'a=1' is shown.

Thanks for reviewing this, @terryjreedy! I've added comments clarifying the reasoning behind the changes that you commented on.

Please note that I'm not trying to be defensive, just explaining my thinking. I'm happy to make any changes you request, @terryjreedy!

Q1: Does SSB keep track of where text is (apparently) frozen, so it only rescans new stuff, and not start at the beginning each time?

No. The major reason for this is the soft-wrapping of lines, which can change when changing the font size, window width, etc. Also, outputs before the iomark can be squeezed/unsqueezed, so they are not totally "frozen".

Note that the sidebar only scans the currently visible lines, not the entire text widget contents.

Also note that getting the display info for lines in the text widget not currently displayed (outside the currently visible area) isn't possible with the current method (.dlineinfo()), and may be impossible (I haven't checked), so avoiding re-calculations for the "frozen" part may be difficult.

To summarize, the currently implementation is straightfoward, robust, and in my testing performs rather well. It might be worth checking on machines with slower CPUs, though.

Q2: If the console tag were removed from a newline and SSB were told to back up to that untagged newline, would it remove the '>>>' it previously inserted? This will also come up if text, such as async output or warnings, needs to be inserted before an entry line.

IIRC such outputs are inserted before the final newline, therefore preserving the newline with the "console" tag. It's position may change due to the contents added before it, which is fine. Regardless, even if the tag were removed and then added back, the sidebar will "just work" as long as it receives a request to update itself, which the current mechanisms should ensure.

Thanks for making the requested changes!

@terryjreedy: please review the changes made to this pull request.

Just did and found it to pass in all repeats (and much quicker!). Also, did you get a chance to look at the issues I raised in a comment further up?

Just did and found it to pass in all repeats.

Thanks, great news!

Also, did you get a chance to look at the issues I raised in a comment further up?

Yes, I've taken a quick look. The first two seem to be related but I haven't gotten to the bottom of the issue yet. Regarding the scroll wheel, I haven't implemented any event handling on the sidebar yet, though I agree that not responding to scrolling is a glaring omission. Since this PR appears to be nearing readiness for merging, I'll try to get to that soon.

Regarding the scroll wheel, I haven't implemented any event handling on the sidebar yet,

Actually, this PR does include code to support scrolling, but it doesn't work on platforms which used mouse buttons 4 and 5 to signify scrolling up/down. I've added the missing bit to support those as well.

Also note that there is currently much code duplication between the line numbers and shell sidebar. I've a got a refactoring of them ready, but I'd like to do that in a separate PR.

./python -m venv ../cpython-venv
../cpython-venv/bin/pip install coverage
../cpython-venv/bin/python -m coverage run -L -m idlelib.idle_test.test_sidebar
../cpython-venv/bin/python -m coverage report | grep -E "Name|---|Lib/idlelib/sidebar.py"

@E-Paine, FYI for IDLE modules, to generate coverage data you should instead run e.g. ... -m idlelib.sidebar, which will run both the unit tests for the module and our manual "htests".

With the latest addition to the tests, I get 89% coverage (including htests). About half of the remaining untested lines are dead lines that will be removed as part of the aforementioned refactoring I'll do in a separate PR.

I've removed some dead code (ShellSidebar.hide_sidebar()) and added tests for mouse scrolling, keyboard interrupt, history, recall, undo and redo.

This brings coverage (with the htest) up to 96%.

Some general comments.

1. This PR is getting overly long and unwieldy. Some comment threads seems to have been duplicated, so that responding on one copy is not reflected on another. Close and open a new PR if you want. In any case, please refresh with git merge upstream/master.

2. On my Windows system, I can detect no difference between spacing of soft- and hard-wrapped lines. This does not really matter though because of squeeze boxes.

3. Themes, tags, and colorizer: Leaving the cursor foreground color aside, theme elements correspond to tags configured with the element's colors. Elements and tags can be divided into syntactical and others, which I call 'structural'. There are also non-theme structure tags that do not affect color. Colorizer itself only colors syntax and for its own purposes only needs to know about syntax tags and its own structure tags. The module could use a few more docstrings and comments (another issue).

In the current implementation, Colorizer.tagdefs includes "ERROR", which it never sets, because other modules depend on a) it being defined here (this could be fixed by defining "ERROR" in editor, along with "sel"), and more importantly, b) it being removed by colorizer when one hits a key after compile fails. Changing this would be harder and maybe not worthwhile.

Colorizer.tagdefs should not define -- and sometimes remove -- structure tags managed by other modules. These include the PyShell tags. Search 'hit' should be in this category, but I'd rather leave verifying that its management is complete, without colorizer, to another time. (If there had been a separate colorizer tagdefs fix issue/pr, I would have checked.)

The PyShell structure tags include "stdin", "stdout", and "stderr". These name are also used for corresponding rpc 'channels'. Is "stdin" used for entered code, input() response, or both? (An outstanding issue is that input response is visibly syntax marked.) I am starting to review live use of the patch again. First new bug is that input prompt and response is initially marked in the sidebar as a text continuation line with '...'.

4. I run coverage with htest lines ignored (as discussed in the test README), along with other exclusions, and conditionals checked both ways, and take the figure given. Is 'htest ignored' what you meant by "(including htests)"?

4. I run coverage with htest lines ignored (as discussed in the test README), along with other exclusions, and conditionals checked both ways, and take the figure given. Is 'htest ignored' what you meant by "(including htests)"?

Thanks for the reminder. I've run the coverage again exactly as specified in the README, and received 93% coverage. Most of non-covered sources lines will be removed via refactoring. I've updated the doc-string.

(I made a Unix-y version of your batch script, @terryjreedy, let me know if you'd like me to put a copy of it somewhere.)

1. This PR is getting overly long and unwieldy. Some comment threads seems to have been duplicated, so that responding on one copy is not reflected on another. Close and open a new PR if you want. In any case, please refresh with git merge upstream/master.

See new PR GH-22682. I'm closing this PR in favor of that.

For future reference:

As far as I can tell, from all of the issues brought up in the comments here so far, the only unresolved issues requiring work in the context of this PR are:

1. Left mouse button interactions with the sidebar (click, double-click, drag). I plan to implement these similarly to the line numbers sidebar in a future PR.
2. An issue with background color of input text brought up by @terryjreedy, which I'm investigating.
3. Minor issue showing ... instead of nothing in the sidebar (or vice-versa) in certain cases where there are syntax errors, brought up by @E-Paine, which I'm investigating.

EDIT:

4. Issue when reading user input, e.g. using input(): The input line(s) are initially marked with ..., which is later removed after entering input is done with. (I plan to resolve this after the initial implementation lands, since it will require more refactoring of existing code.)

Could we possibly add the issue with input to the list?

Getting it "just right" would require more refactoring than I'd like to add to this already large PR.

Is a "part-2" PR a suitable place for such a refactor?

Regarding the background color, here is a comparison of before/after this PR with the background for "normal code or text" changed to pink:

It seems that "stdin" hasn't been using the default background for a while (it's the same on 3.8.5). ISTM that we should simply fix the background for the "stdin" tag. @terryjreedy?

Is a "part-2" PR a suitable place for such a refactor?

The "take 2" PR is just a continuation of this one. After that is merged, followup PRs fixing remaining issues would certainly be welcome.

Could we possibly add the issue with input to the list?

Yes, you're right, I missed that. Good catch!

It seems that "stdin" hasn't been using the default background for a while (it's the same on 3.8.5). ISTM that we should simply fix the background for the "stdin" tag. @terryjreedy?

Actually, taking another look at this, I think the only problem in this example is the white bars until the end of the lines, which are caused by having the '\n' characters at the end of the lines have the "console" tag. Changing that to use the "normal" highlight config results in what I think is the expected behavior:

Documenting testing IDLE is https://bugs.python.org/issue30934.

Changing the tagging of stdout and stderr text is out of scope for this issue. I also, at least at the moment, disagree with the change. Consider

>>> 1/0
Traceback (most recent call last):
  File "<pyshell#1>", line 1, in <module>
    1/0
ZeroDivisionError: division by zero

The 4 traceback lines are evenly tagged from margin to margin with, in my theme, a subtle but distinctive pink background. Without considerable testing, I think I would like a ragged background less. Ditto for truncating the background on just the last line, if the traceback happens to not have a terminating newline.

I am continuing discussion on the new PR, starting with a copy of the 'unfinished'.