-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
bpo-37903: implement shell sidebar mouse interactions #25708
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
bpo-37903: implement shell sidebar mouse interactions #25708
Conversation
I finished #25678 and am off to sleep. Feel free to merge it into master once tests pass again and then merge master in this to get proper indents. I will look at this tomorrow after it has space indents. It would help me write the switch code if you could email me a copy of the #22682 diff with the lines that you think are sidebar specific (mostly just pyshell I believe) marked (with an x on the left, for instance) (or mark the non-sidebar line). |
@terryjreedy, I added a few new commits with tests, better docs and some cleanup. This is now ready as far as I'm concerned.
Sending you that via email. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me, both contex menu 'copy's result in
Exception in Tkinter callback
Traceback (most recent call last):
File "F:\dev\3x\lib\tkinter_init_.py", line 1916, in call
return self.func(*args)
TypeError: ShellSidebar.rmenu_copy_with_prompts_handler() missing 1 required positional argument: 'event'
Can you add "Copy code only"?
Well that's embarrassing! Fixed.
Sure, later today. |
Bug 2: discovered with this patch with master (spaceing) merged, then verified on master without this patch):
Move cursor to either of first 2 inputs, , entire line is recalled. Move to 3rd line and part of line is recalled, After 20+ entries, problem went away. New shell: codecs.ignore_errors. Cursor before '.' and only 'codecs.' is recalled. After, all is recalled. |
Added! |
All 3 'copy's work with a selection. Great. I was thinking that if there were no selection, the whole file should be copied, just like when saving. Opinion? Not crucial. ^-A will select all -- if one knows about it. Most Format menu items seem to make current line the default region, but changing entire file by default is different from copying entire fill. We could ask Guido or anyone on issue what they expect or want. "Copy with prompts" usually puts this in console starting IDLE: or at least 1 time froze IDLE. (The other 2 copys don't have to access sidebar content.) This is why naive testers that don't know what to not do are helpful ;-). I haven't looked at code yet, but history bug is more important. I found that 'by accident' while doing something else. So use in 'real work' is also important. |
This comment has been minimized.
This comment has been minimized.
1 similar comment
I tried switching to a proportional space font. '>>>' is wider than '...' (as right here), but in the sidebar, this is no problem. In fact, I like having the primary prompt emphasized compared to the secondary prompt. Could we tag them with a smaller version of the font in use to narrow the dot spacing? For the text area, one can, if desired, make indent spaces 6, say, to be about '4 chars'. (I am not recommending such fonts, but I know that some beginners, including me, have tried them.) |
The latest commit disables the copy commands in the menu when there is no selection. This avoids all of the related issues mentioned (exceptions and freezing). |
Fixed. The bug was due to ... inconsistent marking of input with the "input" tag (again)! In this case it was auto-completion inserting text with no tags. The fix was to pass the new |
@terryjreedy |
Thanks for making the requested changes! @terryjreedy: please review the changes made to this pull request. |
I'm not sure about this, feels strange. The continuation dots |
Just did a very quick test and think the code only copy behaves strangely. Consider the following examples (where | is the start and end of selection):
|
The format region commands only work on a 'region', a block of contiguous complete lines. They make this obvious after the fact by expanding the selection to a region. I think it reasonable that copy with prompts should do the same thing (copy regions), and it does. But it should also expand the selection to a region to make this obvious. I believe module format has a region find and mark function that we might/should reuse for the copy. I think it reasonable that 'copy only code' should act the same way -- copy a code region, but it also should expand the selection. If it only copies the selection, then it would be a duplicate if there were no output to skip. The point of code copy is to copy code for execution and that would nearly always mean complete lines. The doc entries for these new functions, in the context menu section, can say that they work on regions, as with the region format commands. (And I should check that 'region' is defined. |
Confirmed, good catch! I'll fix this later today. |
Ah yes, I only had problems with dotted names, but only sometimes, because I only used attribute completions but only sometimes. At least I gave a report with enough clues. I probably tested a completion before, but did not follow with enter-recall. I likely only tested tab completion before, which I did not do this time. I least with regard to observable behavior, I think we are close to good enough for now. I will add news and doc updates later after more sleep. |
Sorry, another thing: is it worth adding these new context-menu entries to the 'Edit' menu? |
No, they are shell features, not edit features, and would belong on the Shell menu if anywhere. This should be a separate issue and PR after this is merged. |
Sorry to throw a spanner in the works, but the 'Copy only code' also copies what the user types for |
Presumably, the fix is to not mark response to input() as code input. There is an existing bug that is already is, somehow, so that it gets syntax highlighted. It is a nuisance that 'as', 'for', and 'set get marked in "For we are as one for every set of circumstances.", but not fatal. |
No need to apologize, this is an important observation! Indeed, I think we should hold back on "copy only code" for now due to this.
Indeed, we should mark "code input" differently than input provided between commands. (Sometime, in the future, if anyone ever decides to open that can of worms...) |
For future reference, the main section of code for implementing "copy only code" is: # Find all text with the "stdin" tag in the selected range
code_pieces = []
index = text.index("sel.first")
while tag_range := text.tag_nextrange("stdin", index, "sel.last"):
index = tag_range[1]
code_pieces.append(text.get(*tag_range)) And the main part of the relevant test is: self.assertRegex(copied_text.strip(), r'^if True:\n[ \t]+print\(1\)$') |
The latest commit removes the "copy only code" feature. |
@terryjreedy, the test failure is unrelated (asyncio). |
@pablogsal Pipelines and Travis both failed with Someone must have ignored a failure in the non-required test and merged something that cause failures on subsequent PRs. Or the test failed to fail. One of these has happened before. In any case, I am going to merge this. Good luck getting the releases out. |
I quite fancy giving this a go (no guarantees 😁) if anyone has any tips for me? |
The first step would be to have two tags instead of one, and change the text input interceptor to set the appropriate tag based on the shell's state (search for It would probably best to open a new issue for this to keep the discussion in one place... I will say that IMO this is much lower priority than many of the other outstanding bugs and potential improvements for IDLE. Your welcome to do whatever you like, of course, but if you'd be willing to consider working on other things, I'd be happy to discuss that! |
This is a followup to PR GH-22682, which moved the shell prompts to a sidebar in the IDLE shell.
This implements selection via clicking and dragging on the sidebar, similarly to the line numbers sidebars in editor windows.
Additionally, this adds a right-click context menu to the sidebar, with "Copy" and "Copy with prompts" options. The latter allows copying text identical to what would have been possible without the sidebar, useful for creating doc-tests and interactive shell examples.
This also refactors the code for the shell and line numbers sidebars to share more code.
https://bugs.python.org/issue37903