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

Diff gutter #1487

Merged
merged 1 commit into from
Feb 10, 2020
Merged

Diff gutter #1487

merged 1 commit into from
Feb 10, 2020

Conversation

p-e-w
Copy link
Contributor

@p-e-w p-e-w commented Feb 8, 2020

This PR adds a diff indicator gutter that updates as you type, is completely managed by Micro yet can be controlled by plugins:

diffgutter

Background

The VCS diff plugin does not work with the current version of Micro, but that is not the only problem. Like its cousin vim-gitgutter, it shells out to git diff, parses the output, and then sets gutter markers in Micro, which is messy and slow. As a result, the plugin only shows the diff when it is explicitly invoked, and does not update until it is invoked again.

Solution

This PR takes a different approach by making diffing a first-class function provided by Micro. Plugins only need to set a "diff base", i.e. the text that the buffer is to be diffed against. Everything else, including (re)computing the diff when required and rendering the gutter, is handled automatically by Micro.

This makes creating a Git diff plugin stupidly simple:

VERSION = "1.0.0"

local os = import("os")
local filepath = import("path/filepath")
local shell = import("micro/shell")

function onBufferOpen(buf)
	if (not buf.Type.Scratch) and (buf.Path ~= "") then
		-- check that file exists
		local _, err = os.Stat(buf.AbsPath)
		if err == nil then
			local dirName, fileName = filepath.Split(buf.AbsPath)
			local diffBase, err = shell.ExecCommand("git", "-C", dirName, "show", "HEAD:./" .. fileName)
			if err ~= nil then
				diffBase = buf:Bytes()
			end
			buf:SetDiffBase(diffBase)
		end
	end
end

Only once, when opening the buffer, does this shell out to Git in order to retrieve the contents of the file from the latest revision. The file system is never again touched afterwards, and in fact the plugin is not involved at all anymore. If Git is not available, or the file is not in Git, diffing falls back to using the file as saved to disk as a base, which matches the behavior of Sublime Text.

I have added this as a default plugin since I imagine that most users would want it and Atom, VSCode, and Sublime all provide this functionality by default as well. Of course, it is trivial to adapt this to other version control systems.

Performance

go-diff is absurdly fast. Computing the full diff between two buffers with 10,000 lines each typically takes only about 15 milliseconds. Buffers with fewer than 1000 lines are handled in less than one millisecond. In order to provide an optimal experience for buffers of any size, this implementation takes an adaptive approach depending on the number of lines:

  • Buffers up to 1000 lines are diffed synchronously on every modification. This feels super snappy and avoids unnecessary redraws.
  • Buffers between 1000 and 30,000 lines are diffed asynchronously, with a debounce of 0.5 seconds to minimize work.
  • Buffers larger than 30,000 lines are not diffed at all. This is motivated not primarily by performance concerns but by the fact that go-diff currently has a bug that can produce incorrect results or even panic when generating very large diffs.

Color schemes

To support this feature, color schemes need to provide colors for the keys diff-added, diff-modified, and diff-deleted. Only the foreground color is used for each of these; the background is set automatically to match that of the corresponding line number.

I went through all color schemes shipped with Micro and added appropriate color definitions to each of them. Third-party color schemes will need to be updated separately.

Copy link
Owner

@zyedidia zyedidia left a comment

Choose a reason for hiding this comment

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

Awesome this looks very cool!

b.updateDiffSync()
callback(true)
} else if lineCount < 30000 {
b.updateDiffTimer = time.AfterFunc(500*time.Millisecond, func() {
Copy link
Owner

Choose a reason for hiding this comment

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

What is the point of the updateDiffTimer? This will call the given function in a separate goroutine after 500ms. Wouldn't it be better to simply call the function in a separate goroutine right away? Something like

go func() {
    b.updateDiffSync()
    callback(false)
}()

and then get rid of updateDiffTimer entirely?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a debounce, a rate-limiting mechanism. It consolidates multiple calls to UpdateDiff made within a short period of time into a single diff operation. Note that UpdateDiff returns immediately if a timer is active (i.e., if a diff operation is already scheduled).

As an example, imagine you are editing a file with 20,000 lines (for which a diff operation takes about 30 milliseconds) and typing 10 characters per second. Without the debounce, Micro would perform 10 diff operations per second, taking 300 milliseconds of core time, or 30% of a single CPU core. Additionally, each of these operations would schedule a redraw once it is complete. With the debounce, Micro performs only 2 diff operations and redraws per second, using just 6% of one CPU core.

In other words, this code substantially reduces the CPU usage, at the small price of having to wait a maximum of 0.5 seconds to see diff updates.

@@ -176,6 +176,7 @@ var defaultCommonSettings = map[string]interface{}{
"basename": false,
"colorcolumn": float64(0),
"cursorline": true,
"diffgutter": true,
Copy link
Owner

Choose a reason for hiding this comment

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

I think this should be false by default. I'm open to hearing other opinions though.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer having it true by default as in most other editors out there.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be disabled by default. I feel like micro should be as simple as possible without configuration. Thought that's my philosophy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe Micro should be as useful as possible without configuration. 99% of programmers use Git, and this feature provides an immediate benefit to them (not having to run git diff in order to see changes) without negatively impacting anyone else. I am not aware of any editor that has a built-in diff gutter and disables it by default.

I have carefully designed this feature to be as unobtrusive as possible even for people who don't need/want it. The gutter occupies only a single cell of horizontal space. The impact on CPU and memory usage should be minimal (see above). I definitely think this should be a default feature, especially since (to my knowledge) Micro will be the first terminal-based text editor with a built-in diff gutter and displaying this capability by default could raise its popularity. It definitely was one of the first things I missed when I tried out Micro.

@LevitatingBusinessMan
Copy link
Contributor

I love how this is integrated into micro so well. Seems like a great addition.

@zyedidia zyedidia merged commit de33eac into zyedidia:master Feb 10, 2020
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.

4 participants