{"thread":{"id":"49062","subject":"[RFC PATCH] line-log: clarify [a,b) notation for ranges","startedAt":"2018-08-07T13:54:36Z","lastAt":"2018-08-09T12:50:26Z","messageCount":4,"participants":["Andrei Rybak","Eric Sunshine","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"354734","messageId":"be1dd423-f9de-a436-6127-02cce4ab1977@gmail.com","threadId":"49062","inReplyTo":null,"subject":"[RFC PATCH] line-log: clarify [a,b) notation for ranges","fromName":"Andrei Rybak","fromEmail":"rybak.a.v@gmail.com","sentAt":"2018-08-07T13:54:30Z","receivedAt":"2018-08-07T13:54:36Z","isPatch":true,"sender":{"key":"rybak.a.v@gmail.com","avatar":"https://avatars.githubusercontent.com/u/624072?v=4"},"body":"line-log.[ch] use left-closed, right-open interval logic. Change comment\nand debug output to square brackets+parentheses notation to help\ndevelopers avoid off-by-one errors.\n---\n\nOriginal idea for this change in recent thread about line-log changes:\n\n  https://public-inbox.org/git/9776862d-18b2-43ec-cfeb-829418d4d967@gmail.com/\n\nline-log.c also uses ASCII graphics |---| in some comments, like:\n\n\t/*\n\t * a:         |-------\n\t * b: ------|\n\t */\n\nBut I'm not sure if replacing them with\n\n\t/*\n\t * a:         [-------\n\t * b: ------)\n\t */\n\nwill be helpful.  Another possibility is to update comment for\nrange_set_append_unsafe to read\n\n  /* tack on a _new_ range [a,b) _at the end_ */\n  void range_set_append_unsafe(struct range_set *rs, long a, long b)\n\n line-log.c | 4 ++--\n line-log.h | 2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/line-log.c b/line-log.c\nindex fa9cfd5bd..60f3174ac 100644\n--- a/line-log.c\n+++ b/line-log.c\n@@ -353,7 +353,7 @@ static void dump_range_set(struct range_set *rs, const char *desc)\n \tint i;\n \tprintf(\"range set %s (%d items):\\n\", desc, rs->nr);\n \tfor (i = 0; i < rs->nr; i++)\n-\t\tprintf(\"\\t[%ld,%ld]\\n\", rs->ranges[i].start, rs->ranges[i].end);\n+\t\tprintf(\"\\t[%ld,%ld)\\n\", rs->ranges[i].start, rs->ranges[i].end);\n }\n \n static void dump_line_log_data(struct line_log_data *r)\n@@ -373,7 +373,7 @@ static void dump_diff_ranges(struct diff_ranges *diff, const char *desc)\n \tprintf(\"diff ranges %s (%d items):\\n\", desc, diff->parent.nr);\n \tprintf(\"\\tparent\\ttarget\\n\");\n \tfor (i = 0; i < diff->parent.nr; i++) {\n-\t\tprintf(\"\\t[%ld,%ld]\\t[%ld,%ld]\\n\",\n+\t\tprintf(\"\\t[%ld,%ld)\\t[%ld,%ld)\\n\",\n \t\t       diff->parent.ranges[i].start,\n \t\t       diff->parent.ranges[i].end,\n \t\t       diff->target.ranges[i].start,\ndiff --git a/line-log.h b/line-log.h\nindex e2a5ee7c6..488c86409 100644\n--- a/line-log.h\n+++ b/line-log.h\n@@ -6,7 +6,7 @@\n struct rev_info;\n struct commit;\n \n-/* A range [start,end].  Lines are numbered starting at 0, and the\n+/* A range [start,end).  Lines are numbered starting at 0, and the\n  * ranges include start but exclude end. */\n struct range {\n \tlong start, end;\n-- \n2.18.0\n\n"},{"id":"354774","messageId":"CAPig+cTVEq1AGvtxxonbU4PgqeG0fOy9CSDO7ThjVpfGoJaAJA@mail.gmail.com","threadId":"49062","inReplyTo":"be1dd423-f9de-a436-6127-02cce4ab1977@gmail.com","subject":"Re: [RFC PATCH] line-log: clarify [a,b) notation for ranges","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-07T21:47:02Z","receivedAt":"2018-08-07T21:47:16Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Aug 7, 2018 at 9:54 AM Andrei Rybak <rybak.a.v@gmail.com> wrote:\n> line-log.[ch] use left-closed, right-open interval logic. Change comment\n> and debug output to square brackets+parentheses notation to help\n> developers avoid off-by-one errors.\n> ---\n\nThis seems sensible. There might be some reviewers who suggest\ndifferent notation since \"[...)\" is not universal (see [1]), but I\nthink this is fine.\n\nYou'll want to add your sign-off, of course, when taking this out of RFC.\n\n[1]: https://en.wikipedia.org/wiki/Interval_(mathematics)#Notations_for_intervals\n\n> line-log.c also uses ASCII graphics |---| in some comments, like:\n>\n>         /*\n>          * a:         |-------\n>          * b: ------|\n>          */\n>\n> But I'm not sure if replacing them with\n>\n>         /*\n>          * a:         [-------\n>          * b: ------)\n>          */\n>\n> will be helpful.\n\nThose comments seem to use horizontal ruling to make it clear where\none range ends and another begins, so they already do a good job\nconveying what they represent. Changing them to use \"[\"\" and \")\" might\nmake them less clear.\n\n> Another possibility is to update comment for\n> range_set_append_unsafe to read\n>\n>   /* tack on a _new_ range [a,b) _at the end_ */\n>   void range_set_append_unsafe(struct range_set *rs, long a, long b)\n\nIt shouldn't hurt (though the existing \"_at the end_\" is rather\nsuperfluous since \"append\" in the name says the that already).\n"},{"id":"354862","messageId":"b80df985-03ef-ddb0-37aa-10fd411a1f11@gmail.com","threadId":"49062","inReplyTo":"be1dd423-f9de-a436-6127-02cce4ab1977@gmail.com","subject":"[PATCH v2] line-log: clarify [a,b) notation for ranges","fromName":"Andrei Rybak","fromEmail":"rybak.a.v@gmail.com","sentAt":"2018-08-08T16:26:23Z","receivedAt":"2018-08-08T16:26:29Z","isPatch":true,"sender":{"key":"rybak.a.v@gmail.com","avatar":"https://avatars.githubusercontent.com/u/624072?v=4"},"body":"line-log.[ch] use left-closed, right-open interval logic. Update\ncomments and debug output to use square brackets+parentheses notation\nto help developers avoid off-by-one errors.\n\nSigned-off-by: Andrei Rybak <rybak.a.v@gmail.com>\n---\n\nApplies on top of c0babbe69 (range-set: publish API for re-use by\ngit-blame -L, 2013-08-06). When applied on the original commit 12da1d1f6\n(Implement line-history search (git log -L), 2013-03-28), the conflict\nis in removal of \"static\" from some functions.\n\nChanges since v1:\n\n - reword commit message a bit\n - sign-off\n - add [a,b) to comment of range_set_append_unsafe\n\nI decided not to remove \"_at the end_\", as\n\n  /* tack on a _new_ range [a,b) */\n\nseems a bit less readable to me.\n\n line-log.c | 6 +++---\n line-log.h | 2 +-\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/line-log.c b/line-log.c\nindex fa9cfd5bd..9ab6d51cc 100644\n--- a/line-log.c\n+++ b/line-log.c\n@@ -56,7 +56,7 @@ static void range_set_move(struct range_set *dst, struct range_set *src)\n \tsrc->alloc = src->nr = 0;\n }\n \n-/* tack on a _new_ range _at the end_ */\n+/* tack on a _new_ range [a,b) _at the end_ */\n void range_set_append_unsafe(struct range_set *rs, long a, long b)\n {\n \tassert(a <= b);\n@@ -353,7 +353,7 @@ static void dump_range_set(struct range_set *rs, const char *desc)\n \tint i;\n \tprintf(\"range set %s (%d items):\\n\", desc, rs->nr);\n \tfor (i = 0; i < rs->nr; i++)\n-\t\tprintf(\"\\t[%ld,%ld]\\n\", rs->ranges[i].start, rs->ranges[i].end);\n+\t\tprintf(\"\\t[%ld,%ld)\\n\", rs->ranges[i].start, rs->ranges[i].end);\n }\n \n static void dump_line_log_data(struct line_log_data *r)\n@@ -373,7 +373,7 @@ static void dump_diff_ranges(struct diff_ranges *diff, const char *desc)\n \tprintf(\"diff ranges %s (%d items):\\n\", desc, diff->parent.nr);\n \tprintf(\"\\tparent\\ttarget\\n\");\n \tfor (i = 0; i < diff->parent.nr; i++) {\n-\t\tprintf(\"\\t[%ld,%ld]\\t[%ld,%ld]\\n\",\n+\t\tprintf(\"\\t[%ld,%ld)\\t[%ld,%ld)\\n\",\n \t\t       diff->parent.ranges[i].start,\n \t\t       diff->parent.ranges[i].end,\n \t\t       diff->target.ranges[i].start,\ndiff --git a/line-log.h b/line-log.h\nindex e2a5ee7c6..488c86409 100644\n--- a/line-log.h\n+++ b/line-log.h\n@@ -6,7 +6,7 @@\n struct rev_info;\n struct commit;\n \n-/* A range [start,end].  Lines are numbered starting at 0, and the\n+/* A range [start,end).  Lines are numbered starting at 0, and the\n  * ranges include start but exclude end. */\n struct range {\n \tlong start, end;\n-- \n2.18.0\n\n"},{"id":"354985","messageId":"nycvar.QRO.7.76.6.1808091448260.71@tvgsbejvaqbjf.bet","threadId":"49062","inReplyTo":"CAPig+cTVEq1AGvtxxonbU4PgqeG0fOy9CSDO7ThjVpfGoJaAJA@mail.gmail.com","subject":"Re: [RFC PATCH] line-log: clarify [a,b) notation for ranges","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-08-09T12:50:18Z","receivedAt":"2018-08-09T12:50:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Eric,\n\nOn Tue, 7 Aug 2018, Eric Sunshine wrote:\n\n> On Tue, Aug 7, 2018 at 9:54 AM Andrei Rybak <rybak.a.v@gmail.com> wrote:\n> > line-log.[ch] use left-closed, right-open interval logic. Change comment\n> > and debug output to square brackets+parentheses notation to help\n> > developers avoid off-by-one errors.\n> > ---\n> \n> This seems sensible. There might be some reviewers who suggest\n> different notation since \"[...)\" is not universal (see [1]), but I\n> think this is fine.\n\nIndeed. When I started out studying mathematics, I learned the notation\n[...[ (which makes a lot more sense, if you think about it).\n\nBesides, it's not like we start from a fresh slate. Git is already over a\ndecade old. Our commit ranges are \"half open\", i.e. the exact thing that\nis described here. There's gotta be some precedent in the documentation,\nand introducing something willfully inconsistent is probably a pretty bad\nidea.\n\nCiao,\nDscho\n"}]}