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 Apply Diff feature #20

Merged
merged 1 commit into from
Dec 13, 2023
Merged

Conversation

thedemons
Copy link
Contributor

The technologies:

  • Using the IWpfDifferenceViewerFactoryService.CreateDifferenceView method, we can create a difference view and place it in a window.
  • Using the IAdornmentLayer.AddAdornment, we can make the difference window appears on top of the text editor.
  • Using the ILineTransformSource to extend the space between the lines to compensate for any extra height of the diff window.
  • The IVsTextView.AddCommandFilter can be used to hook up the diff view to the command chain of the host view.
  • An internal method AggregateFocusInterceptor.SetInterceptsAggregateFocus was used to achieve proper focuses for the diff view, this is optional, and the feature works just fine even if that failed.

Check list:

  • Reposition the "Reject" and "Accept" button to make it appears wherever is visible on the screen.
  • Remove the - and + of the diff view to make the code align with the original code
  • Remove internal stuff (still works without the internal method used to achieve proper focuses)
  • Solve the focusing problem
Recording.2023-12-12.070015.mp4

if (docView?.TextView == null) return false;

// any interactions with the `IVsTextView` should be done on the main thread
await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It only throws an exception when right-clicking the diff view. I wonder why it doesn't execute on the main thread, only for the diff view?

Assembly assembly = AppDomain.CurrentDomain.GetAssemblies().SingleOrDefault(assembly => assembly.GetName().Name == name);

Type type = assembly?.GetType("Microsoft.VisualStudio.Text.Editor.AggregateFocusInterceptor");
_fnSetInterceptsAggregateFocus = type?.GetMethod("SetInterceptsAggregateFocus", BindingFlags.Static | BindingFlags.Public);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Try to get the internal SetInterceptsAggregateFocus method using reflection, rather than adding a reference to the assembly.

@thedemons thedemons mentioned this pull request Dec 12, 2023
@thedemons
Copy link
Contributor Author

This PR's complexity makes it hard to comment on the code because we wouldn't be able to see the general picture of it. So I'll leave a short write-up on the inner workings of it here instead.

The core foundations

This relies heavily on a public interface of Visual Studio that lets us seemingly create a difference view without doing the heavy lifting ourselves: CreateDifferenceView.

It is implemented in the class DifferenceViewerFactoryService in the assembly Microsoft.VisualStudio.Platform.VSEditor.dll.

The CreateDifferenceView method simply just constructs a DifferenceViewer class in the same assembly that has the following signature:

internal class DifferenceViewer : IWpfDifferenceViewer, IDifferenceViewer, IPropertyOwner, IDifferenceViewer3, IDifferenceViewer2, IViewSynchronizationManager

Once created, we can display it using its IWpfDifferenceViewer.VisualElement property, which is of type FrameworkElement. It can be displayed anywhere just like a regular WPF UIElement.

In order to display it on top of the text editor, we can create a IAdornmentLayer using the GetAdornmentLayer method of the host IWpfTextView. And then adding our UIElement element to it using the AddAdornment method. This interface is very well documented and there are multiple examples available on GitHub as well as in the official walkthrough.

The icing on the cake

There are four initial problems with this approach:

  1. The default difference view is not editable, even if we explicitly set it to be so.
  2. When we have keyboard focus on the difference view, when a command/shortcut like CTRL+A, CTRL+C, or even the BACKSPACE is executed, it will be received by the host view instead of the difference view.
  3. The selection highlight (i.e. the blue background on selected text) in difference view is always shown as grey-out-of-focus. And the host view doesn't know it loses focus when we focus on the difference view so its selection highlight doesn't be greyed out.
  4. When the difference view height is greater than the original code block height (i.e. more lines of code were added), the adornment will overlap with the host view's lines of code below the original code block.

The solution for each problem is as described below:

1. Make the right view editable

2. Handle commands correctly

  • Quote from this blog:

    Visual Studio uses the command chain design pattern to route commands. Essentially, a linked list is created of different components (all inheriting from IOleCommandTarget) that are interested in listening to commands. Basic commands include arrow key presses, CTRL+Z, and backspace. For just a sampling of the many possible commands see VSConstants.VSStd97CmdID.

  • In short, whichever class implements IVsTextView also implements the IOleCommandTarget to handle commands. The problem is the host view is on top of the chain so the commands were never passed down to our difference view.
  • Using the AddCommandFilter method, we can hook up the command chain of the host view, there for check if our difference view is in focus, and pass the commands to it instead. The code can be found here.

3. Fix the selection highlight

  • Oh boy did I spend a lot of time on fixing this, I've read more internal code than I ever would like to, there are moments of hope and disappointment, finding a new potential solution only to realize that it doesn't work.
  • I can't even really explain what's the root cause of the problem here. But the solution is to use an internal function in Microsoft.VisualStudio.Platform.VSEditor.dll.
  • Snippet from the internal WpfTextView.QueueAggregateFocusCheck():
    internal void QueueAggregateFocusCheck(bool checkForFocus = true)
    {
        /* ... */
        bool flag = false;
        if (checkForFocus)
        {
            flag = _spaceReservationStack.HasAggregateFocus;
            if (!flag && (base.IsKeyboardFocusWithin || _textViewHostBoundOverlayLayer.IsKeyboardFocusWithin))
            {
                DependencyObject dependencyObject = Keyboard.FocusedElement as DependencyObject;
                if (dependencyObject == this || !AggregateFocusInterceptor.GetInterceptsAggregateFocus(dependencyObject))
                {
                    flag = true;
                }
            }
        }
        /* ... */
        _hasAggregateFocus = flag;
        /* ... */
        EventHandler eventHandlers = (_hasAggregateFocus ? this.GotAggregateFocus : this.LostAggregateFocus);
        _factoryService.GuardedOperations.RaiseEvent(this, eventHandlers);
    }
  • You can see from the snippet that we can use AggregateFocusInterceptor.SetInterceptsAggregateFocus(hostView, true) to override the focus check of the host view and let our difference view has the focus instead. The code can be found here.

4. Properly display the adornment

  • We can use the ILineTransformSource to adjust the height of specific text view lines.
  • There isn't much about to talk about except for referencing this official example.
  • The idea is to extend (i.e. increasing the height of) the line to compensate for our difference view height.

Copy link
Collaborator

@fortenforge fortenforge left a comment

Choose a reason for hiding this comment

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

Truly incredible work

@fortenforge fortenforge merged commit 980861a into Exafunction:main Dec 13, 2023
1 check passed
@thedemons thedemons deleted the inline-diffview branch December 13, 2023 04:35
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

Successfully merging this pull request may close these issues.

2 participants