code review 12837044: cmd/gofmt: sort more, remove some duplicate imports

src/pkg/go/token/position.go

Index: src/pkg/go/token/position.go
===================================================================
--- a/src/pkg/go/token/position.go
+++ b/src/pkg/go/token/position.go
@@ -136,6 +136,15 @@
 	f.set.mutex.Unlock()
 }
 
+// RemoveLine removes a line by line number as reported by Position.Line.

[gri, 2013/09/10 20:51:05]

RemoveLine is slightly misleading a name.

This function simply removes a line offset from the table, which is akin to
replacing a newline character (causing the new line) with a space (to not change
the remaining offsets).

This is probably the desired effect for the specific application (we don't want
to re-adjust all positions), but perhaps the function should be named more
aptly? MergeLines comes to mind since this merges two lines - but doesn't really
change the subsequent position offsets. The comments should be adjusted as well.

Also, lines are numbered starting at 1 while the table is 0-indexed; there seems
to be an off-by-1 error somewhere; or the comment needs to be adjusted. An input
of 0 should not be accepted, either.

[josharian, 2013/09/10 23:34:49]

Agreed. I started with RemoveNewline, was unsure about its clarity, and failed
to come up with a better alternative.
I thought about MergeLines, but that feels like it should take two parameters.
Other suggestions?
Yes.
When I looked at this, the first entry in the table was always 0; in practice,
the code below is (I believe) correct and does match the doc. I assumed that
that had been done to avoid having to think about off-by-1 errors when
manipulating the table, and I appreciated it at the time!

+//
+func (f *File) RemoveLine(line int) {
+	f.set.mutex.Lock()
+	copy(f.lines[line:], f.lines[line+1:])
+	f.lines = f.lines[:len(f.lines)-1]
+	f.set.mutex.Unlock()
+}
+
 // SetLines sets the line offsets for a file and returns true if successful.
 // The line offsets are the offsets of the first character of each line;
 // for instance for the content "ab\nc\n" the line offsets are {0, 3}.