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

[CLOSED] Replace in Files can create a broken state with same file in both panes #8540

Open
core-ai-bot opened this issue Aug 30, 2021 · 4 comments

Comments

@core-ai-bot
Copy link
Member

Issue by peterflynn
Wednesday Oct 15, 2014 at 23:14 GMT
Originally opened as adobe/brackets#9569


  1. Split your view
  2. Invoke 'Replace in Files' and enter a search term
  3. Double-click the first result in the bottom pane, adding that file to the working set
  4. Focus the other pane
  5. Click Replace

Result: the file jumps over to the second pane, but the working set still shows it in the first pane. The first pane is blank (without the "no editor" watermark) but its heading still shows the name of the file that is now shown in the second pane.

Expected: file is focused in first pane again, just as if you'd opened it via some other command like Quick Open.

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Wednesday Oct 15, 2014 at 23:15 GMT


There are two bugs at play here:

  • A few parts of the UI, including Replace in Files, call MainViewManager._edit() directly. Its docs say it should only be called by unit tests and File > New. It seems like the other callers could just invoke the normal FILE_OPEN command instead...
  • PR [CLOSED] Saving untitled document corrupts MRU order #9439 has a bug -- the if statement it added in _edit() is reversed, so the optimization is actually inactive in the common case it was trying to speed up (i.e. the code does just as much work as it did before 9439). But in the edge cases where the pane-check is needed, the optimization erroneously kicks in and skips it.

So a simple fix would be to invert the if statement in _edit()... or just remove it since it seems like it doesn't have any significant effect on performance (so the added complexity may not be worth keeping).

Ideally, it would be nice to clean up those stray callers to stop using _edit(), too. Once that's done, we could even consider baking in the optimization instead (since no one should be calling it anymore in a context where that pane-check is necessary).

@core-ai-bot
Copy link
Member Author

Comment by JeffryBooher
Monday Oct 20, 2014 at 20:34 GMT


@peterflynn I was thinking that if there are no files open when the file is added to the working set on the second pane that it should probably go ahead and open the first file in the working set. I searched for "MainViewManager._edit(" and replaced with "FILE_OPEN" and at step 5 in your recipe it adds the remaining files to the second pane but none of them are open because DocumentManager just adds the files to the working set -- it doesn't open them. Should we just do that in the PR to fix this?

@core-ai-bot
Copy link
Member Author

Comment by JeffryBooher
Wednesday Oct 22, 2014 at 17:19 GMT


@peterflynn I have a pr #9636 that fixes this.

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Friday Oct 24, 2014 at 23:23 GMT


Confirmed. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant