- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 33.3k
 
gh-87820: IDLE: fix config disabling tab completion #26421
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| Fix the config dialog breaking tab completion. This is achieved by removing | ||
| the key configuration options for ``<<smart-indent>>`` (Tab), | ||
| ``<<newline-and-indent>>`` (Return, Enter) and ``<<smart-backspace>>`` | ||
| (Backspace) from the configuration mechanism, making them hard-coded | ||
| instead. | ||
| 
         
      Comment on lines
    
      +3
     to 
      +5
    
   
  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These two are not relevant to tab completion. So it we do these together, I think we need to change the PR title (see my issue title revision) and reword this a bit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure the PR/commit title must describe all of the technical changes made. The additional change to the  Having this detail in the NEWS entry, and possibly in the body of the commit message, seems right to me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be clear, I now think that the existing PR title is good, since it describes the only significant bug being fixed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Today I think the title is fine as is. The message that follows can say more.  | 
||
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.
The only function reason to keep these fixed keys separate from those that follow is that the pseudoevent -- handler bindings are done above for these and below for the existing fixed key keys. And this is related to where the handler are defined. So keep the separation for now.