Commit Graph

4 Commits

Author SHA1 Message Date
Alan Donovan d96b2388c6 internal/diff: simplify API, break span dependency
diff.TextEdit (now called simply Edit) no longer has a Span,
and no longer depends on the span package, which is really
part of gopls. Instead, it records only start/end byte
offsets within an (implied) file.

The diff algorithms have been simplified to avoid the need to
map offsets to line/column numbers (e.g. using a token.File).
All the conditions actually needed by the logic can be derived
by local string operations on the source text.

This change will allow us to move the span package into the
gopls module.

I was expecting that gopls would want to define its own
Span-augmented TextEdit type but, surprisingly, diff.Edit
is quite convenient to use throughout the entire repo:
in all places in gopls that manipulate Edits, the implied
file is obvious. In most cases, less conversion boilerplate
is required than before.

API Notes:
- diff.TextEdit -> Edit (it needn't be text)
- diff.ApplyEdits -> Apply
- source.protocolEditsFromSource is now private

Change-Id: I4d7cef078dfbd189b4aef477f845db320af6c5f6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/436781
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
2022-10-05 20:32:15 +00:00
Alan Donovan 9856077059 internal/diff: abolish errors
Computing the difference between two strings is logically an
infallible operation. This change makes the code reflect that. The
actual failures were unreachable given consistent inputs, but that was
hard to see from the complexity of the logic surrounding span.Span.
(The problem only occurs when converting offsets beyond the end of the
file to Spans, but the code preserves the integrity of offsets.)

gopls' "old" hooks.ComputeEdits impl (based on sergi/go-diff) now
reports a bug and returns a single diff for the entire file if it
panics.

Also, first steps towards simpler API and a
reusable diff package in x/tools:

- add TODO for new API. In particular, the diff package shouldn't care
  about filenames, spans, and URIs. These are gopls concerns.
- diff.String is the main diff function.
- diff.Unified prints edits in unified form;
  all its internals are now hidden.
- the ComputeEdits func type is moved to gopls (source.DiffFunction)
- remove all non-critical uses of myers.ComputeEdits. The only
  remaining one is in gopls' defaults, but perhaps that gets
  overridden by the default GoDiff setting in hooks, to BothDiffs
  (sergi + pjw impls), so maybe it's now actually unused in practice?

Change-Id: I6ceb5c670897abbf285b243530a7372dfa41edf6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/436778
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
2022-10-05 20:30:46 +00:00
Alan Donovan 55e5cff611 internal/diff: unified: match GNU diff for empty file
GNU diff -u prints -0,0 for the special case of inserting
into an empty file. (I don't understand why this case is
special, but it's fair to assume it has been debugged;
perhaps patch(1) has a bug.)

This change matches that special behavior, and adds a test.
It also adds a (disabled) test for another bug I encountered.

Also, remove some overzealous uses of t.Helper() that served
to obscure the failure.

Change-Id: I3e0c389c478cde45163353130e36f2f8741a8659
Reviewed-on: https://go-review.googlesource.com/c/tools/+/436782
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Alan Donovan <adonovan@google.com>
2022-10-03 15:32:10 +00:00
Robert Findley b15dac2b88 gopls: migrate internal/lsp to gopls/internal/lsp
This CL was created using the following commands:

    ./gopls/internal/migrate.sh
    git add .
    git codereview gofmt

For golang/go#54509

Change-Id: Iceeec602748a5e6f609c3ceda8d19157e5c94009
Reviewed-on: https://go-review.googlesource.com/c/tools/+/426796
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-09-07 16:44:44 +00:00