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.

Sign up for GitHub

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

Jump to bottom

Fix ctrlf search completion repositioning the window #90 #92

Closed
wants to merge 3 commits into from
Closed

Fix ctrlf search completion repositioning the window #90 #92

wants to merge 3 commits into from

Conversation

rileyrg
Copy link

@rileyrg rileyrg commented Apr 10, 2021 •
edited
Loading

#90

@rileyrg rileyrg mentioned this pull request Apr 10, 2021
Closed
@raxod502
Copy link
Member

Well, so, amusingly enough, we did have exactly this code previously, and then it was reported that that was causing problems with split windows, so I removed it: #81

I think we need to identify under what circumstances this line fixes a bug, and under what circumstances it causes a bug, so that we can make it conditional. Otherwise we'll just end up adding and removing this line ad infinitum.

I guess until it's discovered what's causing the issue, we could at least make a user option so that people could set it to the appropriate value for their configuration, perhaps with a note in the docstring suggesting they add their feedback to this issue thread.

@raxod502
Copy link
Member

This thread is being closed automatically by Tidier because it is labeled with "waiting on response" and has not seen any activity for 90 days. But don't worry—if you have any information that might advance the discussion, leave a comment and I will be happy to reopen the thread :)

@raxod502 raxod502 closed this Jul 19, 2021
@raxod502
Copy link
Member

Sorry, I didn't see that you had added a user option. (I don't get email notifications for pushed commits.)

raxod502
raxod502 reviewed Jul 24, 2021
View reviewed changes
Copy link
Member

@raxod502 raxod502 left a comment

Choose a reason for hiding this comment

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

Comments:

  • ctrlf--restore-final-window-start-flag should be ctrlf-restore-final-window-start-flag since it's part of the public API.
  • README and changelog should be updated.

If you open a new PR or reopen this one I'm happy to merge it.

NicholasBHubbard added a commit to NicholasBHubbard/ctrlf that referenced this pull request Apr 23, 2022
@NicholasBHubbard
Keep centered on return with ctrlf-auto-recenter. radian-software#90, r…
24ed8d6 
…adian-software#81, radian-software#92
@NicholasBHubbard NicholasBHubbard mentioned this pull request Apr 23, 2022
Merged
raxod502 added a commit that referenced this pull request May 1, 2022
@NicholasBHubbard @raxod502
Keep centered on return with ctrlf-auto-recenter. #90, #81, #92 (#113)
efe9534 
* Keep centered on return with ctrlf-auto-recenter. #90, #81, #92

* Update changelog

* Hack indentation

Co-authored-by: Radon Rosborough <radon.neon@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raxod502 raxod502 raxod502 left review comments

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rileyrg @raxod502