diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index 45648e597b..b52582638a 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -5386,10 +5386,10 @@ The `CommentRange` entity describes the range of an inline comment. [options="header",cols="1,^1,5"] |=========================== |Field Name ||Description -|`start_line` ||The start line number of the range. -|`start_character` ||The character position in the start line. -|`end_line` ||The end line number of the range. -|`end_character` ||The character position in the end line. +|`start_line` ||The start line number of the range. (1-based, inclusive) +|`start_character` ||The character position in the start line. (0-based, inclusive) +|`end_line` ||The end line number of the range. (1-based, exclusive) +|`end_character` ||The character position in the end line. (0-based, exclusive) |=========================== [[commit-info]] diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/Comment.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/Comment.java index 5e9ca94d18..2225a99368 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/Comment.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/Comment.java @@ -29,7 +29,7 @@ public abstract class Comment { public String path; public Side side; public Integer parent; - public Integer line; + public Integer line; // value 0 or null indicates a file comment, normal lines start at 1 public Range range; public String inReplyTo; public Timestamp updated; @@ -37,15 +37,15 @@ public abstract class Comment { public Boolean unresolved; public static class Range { - public int startLine; - public int startCharacter; - public int endLine; - public int endCharacter; + public int startLine; // 1-based, inclusive + public int startCharacter; // 0-based, inclusive + public int endLine; // 1-based, exclusive + public int endCharacter; // 0-based, exclusive public boolean isValid() { - return startLine >= 0 + return startLine > 0 && startCharacter >= 0 - && endLine >= 0 + && endLine > 0 && endCharacter >= 0 && startLine <= endLine && (startLine != endLine || startCharacter <= endCharacter); diff --git a/gerrit-extension-api/src/test/java/com/google/gerrit/extensions/client/RangeTest.java b/gerrit-extension-api/src/test/java/com/google/gerrit/extensions/client/RangeTest.java index f209e7c17a..9695933679 100644 --- a/gerrit-extension-api/src/test/java/com/google/gerrit/extensions/client/RangeTest.java +++ b/gerrit-extension-api/src/test/java/com/google/gerrit/extensions/client/RangeTest.java @@ -62,6 +62,30 @@ public class RangeTest { assertThat(range).isInvalid(); } + @Test + public void zeroStartLineResultsInInvalidRange() { + Comment.Range range = createRange(0, 2, 19, 10); + assertThat(range).isInvalid(); + } + + @Test + public void zeroEndLineResultsInInvalidRange() { + Comment.Range range = createRange(13, 2, 0, 10); + assertThat(range).isInvalid(); + } + + @Test + public void zeroStartCharacterResultsInValidRange() { + Comment.Range range = createRange(13, 0, 19, 10); + assertThat(range).isValid(); + } + + @Test + public void zeroEndCharacterResultsInValidRange() { + Comment.Range range = createRange(13, 31, 19, 0); + assertThat(range).isValid(); + } + @Test public void startLineGreaterThanEndLineResultsInInvalidRange() { Comment.Range range = createRange(20, 2, 19, 10); diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Comment.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Comment.java index b2c72d9568..cadd52cc0c 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Comment.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Comment.java @@ -131,10 +131,10 @@ public class Comment { } public static class Range { - public int startLine; - public int startChar; - public int endLine; - public int endChar; + public int startLine; // 1-based, inclusive + public int startChar; // 0-based, inclusive + public int endLine; // 1-based, exclusive + public int endChar; // 0-based, exclusive public Range(Range r) { this(r.startLine, r.startChar, r.endLine, r.endChar);