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

Add control to open file shown in "replace preview" editor when performing project-wide find/replace #35337

Open
Tracked by #221516
komputerwiz opened this issue Sep 28, 2017 · 15 comments
Assignees
Labels
feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities search Search widget and operation issues
Milestone

Comments

@komputerwiz
Copy link

While performing a project-wide find/replace, opening an occurrence from the search sidebar opens a "replace preview" diff editor. This view is not editable, so if the user would like to edit the displayed file, the only current way to do so is manually either from the explorer sidebar or the quick-open palette.

Per @roblourens' suggestion in #35323, "There should be a working Open File button that always appears, git repo or not, in the replace diff editor."

@vscodebot vscodebot bot added the search Search widget and operation issues label Sep 28, 2017
@davidgiven
Copy link

I agree, but could we also make the replace preview view editable, allowing us to make quick fixes directly?

@roblourens roblourens added the feature-request Request for new features or functionality label Oct 14, 2017
@komputerwiz
Copy link
Author

I had the same thought upon realizing neither side of the "replace preview" was editable, which is when I noticed the "open file" button at the top and found #35323 when clicking that button did nothing. The root "expectation failure" was indeed that neither side of the replace preview was editable.

In the interest of making the developers' lives easy (which I appreciate since I, too, am a developer 😛 ), I thought it would be easier to add functionality to the existing "open file" button than to make the replace preview editable. At your suggestion, @davidgiven, I have further entertained the idea of making the diff editors editable. Here are my thoughts:

The git diff editor is editable because there are two separate files that are being compared and edited: one in the working tree and the other in the git index. This problem becomes harder when there is only one file, but two versions of it: Which buffer should be editable? What happens when one is edited? What if the updated contents now match the search pattern? Nonetheless, the fact that git diff editors are editable is a good sign that it could be doable for the replace preview diff editor.

The replace preview diff editor is appears to be implemented by opening two buffers with contents of the given file and then performing the find/replace actions on the "right-hand" buffer. It seems like it should be straightforward to make the "original" (left-hand) buffer editable given the available diff editor options:

Disclaimer: I am not at all familiar with the VS Code codebase, so any observations and suggestions here are speculation. Code snippets and references are based on master @ a4ae837, dated 2017-10-16 17:38Z.

It seems that the replace preview window is opened in the following function at src/vs/workbench/parts/search/browser/replaceService.ts:139:

public openReplacePreview(element: FileMatchOrMatch, preserveFocus?: boolean, sideBySide?: boolean, pinned?: boolean): TPromise<any> {
  /* __GDPR__
    "replace.open.previewEditor" : {}
  */
  this.telemetryService.publicLog('replace.open.previewEditor');
  const fileMatch = element instanceof Match ? element.parent() : element;

  return this.editorService.openEditor({
    leftResource: fileMatch.resource(),
    rightResource: toReplaceResource(fileMatch.resource()),
    label: nls.localize('fileReplaceChanges', "{0} ↔ {1} (Replace Preview)", fileMatch.name(), fileMatch.name()),
    options: {
      preserveFocus,
      pinned,
      revealIfVisible: true
    }
  }).then(editor => {
    this.updateReplacePreview(fileMatch).then(() => {
      let editorControl = (<IDiffEditor>editor.getControl());
      if (element instanceof Match) {
        editorControl.revealLineInCenter(element.range().startLineNumber, ScrollType.Immediate);
      }
    });
  }, errors.onUnexpectedError);
}

The diff editor configuration (src/vs/editor/common/config/editorOptions.ts:544) accepted by the diff editor widget (src/vs/editor/browser/widget/diffEditorWidget.ts) seems to have an originalEditable option:

/**
 * Configuration options for the diff editor.
 */
export interface IDiffEditorOptions extends IEditorOptions {
  /**
   * Allow the user to resize the diff editor split view.
   * Defaults to true.
   */
  enableSplitViewResizing?: boolean;
  /**
   * Render the differences in two side-by-side editors.
   * Defaults to true.
   */
  renderSideBySide?: boolean;
  /**
   * Compute the diff by ignoring leading/trailing whitespace
   * Defaults to true.
   */
  ignoreTrimWhitespace?: boolean;
  /**
   * Render +/- indicators for added/deleted changes.
   * Defaults to true.
   */
  renderIndicators?: boolean;
  /**
   * Original model should be editable?
   * Defaults to false.
   */
  originalEditable?: boolean;
}

If there was a way to set this option from inside openReplacePreview(), then this might solve the problem. I wish it were as simple as adding it to the call to editorService.openEditor(), but the editor options interface referenced in the type signature to openEditor() is defined in src/vs/platform/editor/common/editor.ts and therefore disjoint from the options that are passed to the diff editor widget.

I will defer to @roblourens or any of the other VS Code devs: they have deeper insight into how things actually work and how to create an editable diff editor.

@davidgiven
Copy link

To answer your questions as to expectations: I would expect the right-hand-side to be editable (as it's a before-and-after view and I expect to have to fix things after a replace has broken something). I would not expect changes there to affect the 'before' side of the editor until I save them, at which point I'd see the find-and-replace view update to reflect the new contents. That's how other IDEs I've used work.

I agree that needs more careful thought, as that's not actually how VSCode works --- but that's what I expect, deep down; I'm always slightly startled when I see the find-and-replace view update live as I edit the files. It may be better to have the 'before' view editable. That would require replaces to be instantly updated there (I forget whether it is right now). The way the 'before' view always reflects the current state of the file.

@roblourens
Copy link
Member

roblourens commented Oct 17, 2017

Yeah, making that editable would take some thought - it's not clear to me that the RHS should be editable because it's showing a theoretical proposed view for every replacement in the file. If I make an edit and save the file, does that treat the full diff as unsaved changes, and apply every replacement in the file?

And for the LHS, would be weird to have unsaved changes on the left, then apply a replace on top of that.

Would take some thought on how this should work in any case.

@komputerwiz
Copy link
Author

@roblourens: My thoughts exactly on making the RHS editable since that version of the file technically does not exist yet.

If there is a file open with unsaved changes, does VS Code's project-wide search read at the file on disk (like calling out to ag or grep would) or does it read the already-open, unsaved buffer? If the latter is the case, then having unsaved changes on the LHS should keep the file in a consistent state through the search/replace process: the RHS would just have to be updated each time the LHS changes.

For what it's worth, applying replacements puts the file in a dirty state anyway.

@roblourens
Copy link
Member

It still feels like it would be unexpected for it to be editable. I think find/replace should just perform the specified find/replace, and any other changes can be made in the editor. But we should fix the original issue of having a control to open the file.

@davidgiven
Copy link

Pretty much every other IDE I've used allows comparing diff views (with the RHS being editable).

@roblourens
Copy link
Member

Which ones? I can't think of one among the editors I've used.

@davidgiven
Copy link

Actually, I think I'm wrong about that. I think I'm confusing the find-and-replace view with the VCS diff view.

That does still kinda suggest that the split page is the wrong UI for this job, based on the principle that things that are different should look different... it's so similar to the diff view that I just naturally expect it to behave the same way.

I still think something's wrong here (maybe just show the prospective changes as special highlights in an ordinary live editor, similar to how they're shown in the find-and-replace index view?) but that's getting increasingly out of scope so I'll shut up about it now.

@roblourens
Copy link
Member

I think the find/replace diff view looks better if you use the inline diff, like I do. And actually there's a feature request to show in the sidebar view of it, an actual diff, rather than the fake diff we show now.

@rwxrob
Copy link

rwxrob commented Dec 3, 2018

I definitely need this. After finding the file I have to dig extensively through the explorer to find the file and edit it. This prevents quick use of global search to open files for editing (much like spotlight and cortana allow). This is a use case in addition to the regular find and replace that leverages the functionality of global search to facilitate quick file edits.

@cdyson37
Copy link

Implementing this would be great! There's something a bit painful about having to search for a file that's open in front of me, because I've seen something small that separately needs changing.

@scanzy
Copy link

scanzy commented Sep 26, 2019

I also think that it would be very userful to have an "Open file" button, however I found this workaround.

Workaround

  1. click the arrow near the the search textbox to enter search only mode (without replace)
  2. click entries in the list, now the file should open normally (not in the diff editor)

Before (find & replace mode):
Cattura

After (find only mode):
Cattura2

@roblourens roblourens added the help wanted Issues identified as good community contribution opportunities label Oct 27, 2019
@roblourens roblourens added this to the Backlog milestone Oct 27, 2019
@andreamah andreamah modified the milestones: Backlog, On Deck Dec 5, 2023
@meganrogge
Copy link
Contributor

I would like this too - sometimes I feel like I'm trapped in that editor, which detracts from the experience overall that is otherwise great.

@meganrogge
Copy link
Contributor

Just adding an open file button to the toolbar would be great IMO

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities search Search widget and operation issues
Projects
None yet
Development

No branches or pull requests

8 participants