From 0a9f6b5da62000833fd92f56cfb53d712f70d8c7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Mur=C3=A9?= <batolettre@gmail.com>
Date: Tue, 14 Jul 2020 23:01:57 +0200
Subject: [PATCH] bug: improve the comment Id code structure
---
bug/comment.go | 48 +++++++++++++++++++-----------
bug/comment_test.go | 27 +++++++++--------
bug/op_add_comment.go | 2 +-
bug/op_create.go | 2 +-
bug/snapshot.go | 5 ++++
cache/repo_cache_bug.go | 48 ++++++++++++++++++++++++++++++
commands/comment_edit.go | 8 ++---
commands/select/select.go | 24 ---------------
doc/man/git-bug-comment-edit.1 | 36 ++++++++++++++++++++++
doc/man/git-bug-comment.1 | 2 +-
doc/md/git-bug_comment.md | 1 +
doc/md/git-bug_comment_edit.md | 24 +++++++++++++++
misc/bash_completion/git-bug | 29 ++++++++++++++++++
misc/powershell_completion/git-bug | 8 +++++
misc/zsh_completion/git-bug | 10 +++++++
15 files changed, 213 insertions(+), 61 deletions(-)
create mode 100644 doc/man/git-bug-comment-edit.1
create mode 100644 doc/md/git-bug_comment_edit.md
diff --git a/bug/comment.go b/bug/comment.go
index f79e1293..1a9ca05a 100644
--- a/bug/comment.go
+++ b/bug/comment.go
@@ -1,6 +1,8 @@
package bug
import (
+ "strings"
+
"github.com/dustin/go-humanize"
"github.com/MichaelMure/git-bug/entity"
@@ -21,8 +23,6 @@ type Comment struct {
UnixTime timestamp.Timestamp
}
-const compiledCommentIdFormat = "BCBCBCBBBCBBBBCBBBBCBBBBCBBBBCBBBBCBBBBC"
-
// Id return the Comment identifier
func (c Comment) Id() entity.Id {
if c.id == "" {
@@ -33,34 +33,48 @@ func (c Comment) Id() entity.Id {
return c.id
}
-func DeriveCommentId(bugId entity.ID, commentId entity.ID) entity.ID {
- commentIdString := commentId
- bugIdString := bugId
- id := ""
+const compiledCommentIdFormat = "BCBCBCBBBCBBBBCBBBBCBBBBCBBBBCBBBBCBBBBC"
+
+// DeriveCommentId compute a merged Id for a comment holding information from
+// both the Bug's Id and the Comment's Id. This allow to later find efficiently
+// a Comment because we can access the bug directly instead of searching for a
+// Bug that has a Comment matching the Id.
+//
+// To allow the use of an arbitrary length prefix of this merged Id, Ids from Bug
+// and Comment are interleaved with this irregular pattern to give the best chance
+// to find the Comment even with a 7 character prefix.
+//
+// A complete merged Id hold 30 characters for the Bug and 10 for the Comment,
+// which give a key space of 36^30 for the Bug (~5 * 10^46) and 36^10 for the
+// Comment (~3 * 10^15). This asymmetry assume a reasonable number of Comment
+// within a Bug, while still allowing for a vast key space for Bug (that is, a
+// globally merged bug database) with a low risk of collision.
+func DeriveCommentId(bugId entity.Id, commentId entity.Id) entity.Id {
+ var id strings.Builder
for _, char := range compiledCommentIdFormat {
if char == 'B' {
- id += string(bugIdString[0])
- bugIdString = bugIdString[1:]
+ id.WriteByte(bugId[0])
+ bugId = bugId[1:]
} else {
- id += string(commentIdString[0])
- commentIdString = commentIdString[1:]
+ id.WriteByte(commentId[0])
+ commentId = commentId[1:]
}
}
- return id
+ return entity.Id(id.String())
}
func SplitCommentId(prefix string) (bugPrefix string, commentPrefix string) {
- commentIdPrefix := ""
- bugIdPrefix := ""
+ var bugIdPrefix strings.Builder
+ var commentIdPrefix strings.Builder
- for i, char := range id {
+ for i, char := range prefix {
if compiledCommentIdFormat[i] == 'B' {
- bugIdPrefix += string(char)
+ bugIdPrefix.WriteRune(char)
} else {
- commentIdPrefix += string(char)
+ commentIdPrefix.WriteRune(char)
}
}
- return bugIdPrefix, commentIdPrefix
+ return bugIdPrefix.String(), commentIdPrefix.String()
}
// FormatTimeRel format the UnixTime of the comment for human consumption
diff --git a/bug/comment_test.go b/bug/comment_test.go
index c2b29d93..a73ae43f 100644
--- a/bug/comment_test.go
+++ b/bug/comment_test.go
@@ -4,21 +4,24 @@ import (
"testing"
"github.com/stretchr/testify/assert"
+
+ "github.com/MichaelMure/git-bug/entity"
)
-func TestCompileUnpackCommentId(t *testing.T) {
- id1 := "abcdefghijklmnopqrstuvwxyz1234"
- id2 := "ABCDEFGHIJ"
- expectedId := "aAbBcCdefDghijEklmnFopqrGstuvHwxyzI1234J"
+func TestCommentId(t *testing.T) {
+ bugId := entity.Id("abcdefghijklmnopqrstuvwxyz1234__________")
+ opId := entity.Id("ABCDEFGHIJ______________________________")
+ expectedId := entity.Id("aAbBcCdefDghijEklmnFopqrGstuvHwxyzI1234J")
- compiledId := CompileCommentId(id1, id2)
- assert.Equal(t, expectedId, compiledId)
+ mergedId := DeriveCommentId(bugId, opId)
+ assert.Equal(t, expectedId, mergedId)
- unpackedId1, unpackedId2 := UnpackCommentId(compiledId)
- assert.Equal(t, id1, unpackedId1)
- assert.Equal(t, id2, unpackedId2)
+ // full length
+ splitBugId, splitCommentId := SplitCommentId(mergedId.String())
+ assert.Equal(t, string(bugId[:30]), splitBugId)
+ assert.Equal(t, string(opId[:10]), splitCommentId)
- unpackedId1, unpackedId2 = UnpackCommentId(expectedId[:6])
- assert.Equal(t, unpackedId1, id1[:3])
- assert.Equal(t, unpackedId2, id2[:3])
+ splitBugId, splitCommentId = SplitCommentId(string(expectedId[:6]))
+ assert.Equal(t, string(bugId[:3]), splitBugId)
+ assert.Equal(t, string(opId[:3]), splitCommentId)
}
diff --git a/bug/op_add_comment.go b/bug/op_add_comment.go
index 9a513f52..df426ee0 100644
--- a/bug/op_add_comment.go
+++ b/bug/op_add_comment.go
@@ -36,7 +36,7 @@ func (op *AddCommentOperation) Apply(snapshot *Snapshot) {
snapshot.addActor(op.Author)
snapshot.addParticipant(op.Author)
- commentId := entity.Id(CompileCommentId(snapshot.Id().String(), op.Id().String()))
+ commentId := DeriveCommentId(snapshot.Id(), op.Id())
comment := Comment{
id: commentId,
Message: op.Message,
diff --git a/bug/op_create.go b/bug/op_create.go
index 19ce4834..54bb61aa 100644
--- a/bug/op_create.go
+++ b/bug/op_create.go
@@ -39,7 +39,7 @@ func (op *CreateOperation) Apply(snapshot *Snapshot) {
snapshot.Title = op.Title
- commentId := entity.Id(CompileCommentId(snapshot.Id().String(), op.Id().String()))
+ commentId := DeriveCommentId(snapshot.Id(), op.Id())
comment := Comment{
id: commentId,
Message: op.Message,
diff --git a/bug/snapshot.go b/bug/snapshot.go
index 11df04b2..0005b930 100644
--- a/bug/snapshot.go
+++ b/bug/snapshot.go
@@ -28,6 +28,11 @@ type Snapshot struct {
// Return the Bug identifier
func (snap *Snapshot) Id() entity.Id {
+ if snap.id == "" {
+ // simply panic as it would be a coding error
+ // (using an id of a bug not stored yet)
+ panic("no id yet")
+ }
return snap.id
}
diff --git a/cache/repo_cache_bug.go b/cache/repo_cache_bug.go
index 30692363..6c5be86f 100644
--- a/cache/repo_cache_bug.go
+++ b/cache/repo_cache_bug.go
@@ -3,6 +3,7 @@ package cache
import (
"bytes"
"encoding/gob"
+ "errors"
"fmt"
"os"
"path"
@@ -205,6 +206,53 @@ func (c *RepoCache) resolveBugMatcher(f func(*BugExcerpt) bool) (entity.Id, erro
return matching[0], nil
}
+// ResolveComment search for a Bug/Comment combination matching the merged
+// bug/comment Id prefix. Returns the Bug containing the Comment and the Comment's
+// Id.
+func (c *RepoCache) ResolveComment(prefix string) (*BugCache, entity.Id, error) {
+ bugPrefix, _ := bug.SplitCommentId(prefix)
+ bugCandidate := make([]entity.Id, 0, 5)
+
+ // build a list of possible matching bugs
+ c.muBug.RLock()
+ for _, excerpt := range c.bugExcerpts {
+ if excerpt.Id.HasPrefix(bugPrefix) {
+ bugCandidate = append(bugCandidate, excerpt.Id)
+ }
+ }
+ c.muBug.RUnlock()
+
+ matchingBugIds := make([]entity.Id, 0, 5)
+ matchingCommentId := entity.UnsetId
+ var matchingBug *BugCache
+
+ // search for matching comments
+ // searching every bug candidate allow for some collision with the bug prefix only,
+ // before being refined with the full comment prefix
+ for _, bugId := range bugCandidate {
+ b, err := c.ResolveBug(bugId)
+ if err != nil {
+ return nil, entity.UnsetId, err
+ }
+
+ for _, comment := range b.Snapshot().Comments {
+ if comment.Id().HasPrefix(prefix) {
+ matchingBugIds = append(matchingBugIds, bugId)
+ matchingBug = b
+ matchingCommentId = comment.Id()
+ }
+ }
+ }
+
+ if len(matchingBugIds) > 1 {
+ return nil, entity.UnsetId, entity.NewErrMultipleMatch("bug/comment", matchingBugIds)
+ } else if len(matchingBugIds) == 0 {
+ return nil, entity.UnsetId, errors.New("comment doesn't exist")
+ }
+
+ return matchingBug, matchingCommentId, nil
+}
+
// QueryBugs return the id of all Bug matching the given Query
func (c *RepoCache) QueryBugs(q *query.Query) []entity.Id {
c.muBug.RLock()
diff --git a/commands/comment_edit.go b/commands/comment_edit.go
index 856e13c7..61132967 100644
--- a/commands/comment_edit.go
+++ b/commands/comment_edit.go
@@ -3,7 +3,6 @@ package commands
import (
"github.com/spf13/cobra"
- _select "github.com/MichaelMure/git-bug/commands/select"
"github.com/MichaelMure/git-bug/input"
)
@@ -17,7 +16,7 @@ func newCommentEditCommand() *cobra.Command {
options := commentEditOptions{}
cmd := &cobra.Command{
- Use: "edit <id>",
+ Use: "edit <commentid>",
Short: "Edit an existing comment on a bug.",
Args: cobra.ExactArgs(1),
PreRunE: loadBackendEnsureUser(env),
@@ -40,8 +39,7 @@ func newCommentEditCommand() *cobra.Command {
}
func runCommentEdit(env *Env, opts commentEditOptions, args []string) error {
- b, c, err := _select.ResolveComment(env.backend, args[0])
-
+ b, commentId, err := env.backend.ResolveComment(args[0])
if err != nil {
return err
}
@@ -64,7 +62,7 @@ func runCommentEdit(env *Env, opts commentEditOptions, args []string) error {
}
}
- _, err = b.EditComment(c, opts.message)
+ _, err = b.EditComment(commentId, opts.message)
if err != nil {
return err
}
diff --git a/commands/select/select.go b/commands/select/select.go
index cb427c05..cf861fcc 100644
--- a/commands/select/select.go
+++ b/commands/select/select.go
@@ -69,30 +69,6 @@ func ResolveBug(repo *cache.RepoCache, args []string) (*cache.BugCache, []string
return nil, nil, ErrNoValidId
}
-func ResolveComment(repo *cache.RepoCache, fullId string) (*cache.BugCache, entity.Id, error) {
- bugId, _ := bug.SplitCommentId(fullId)
- b, _, err := ResolveBug(repo, []string{bugId})
- if err != nil {
- return nil, entity.UnsetId, err
- }
-
- matching := make([]entity.Id, 0, 5)
-
- for _, comment := range b.Snapshot().Comments {
- if comment.Id().HasPrefix(fullId) {
- matching = append(matching, comment.Id())
- }
- }
-
- if len(matching) > 1 {
- return nil, entity.UnsetId, entity.NewErrMultipleMatch("comment", matching)
- } else if len(matching) == 0 {
- return nil, entity.UnsetId, errors.New("comment doesn't exist")
- }
-
- return b, matching[0], nil
-}
-
// Select will select a bug for future use
func Select(repo *cache.RepoCache, id entity.Id) error {
selectPath := selectFilePath(repo)
diff --git a/doc/man/git-bug-comment-edit.1 b/doc/man/git-bug-comment-edit.1
new file mode 100644
index 00000000..e20621c1
--- /dev/null
+++ b/doc/man/git-bug-comment-edit.1
@@ -0,0 +1,36 @@
+.nh
+.TH GIT\-BUG(1)Apr 2019
+Generated from git\-bug's source code
+
+.SH NAME
+.PP
+git\-bug\-comment\-edit \- Edit an existing comment on a bug.
+
+
+.SH SYNOPSIS
+.PP
+\fBgit\-bug comment edit [flags]\fP
+
+
+.SH DESCRIPTION
+.PP
+Edit an existing comment on a bug.
+
+
+.SH OPTIONS
+.PP
+\fB\-F\fP, \fB\-\-file\fP=""
+ Take the message from the given file. Use \- to read the message from the standard input
+
+.PP
+\fB\-m\fP, \fB\-\-message\fP=""
+ Provide the new message from the command line
+
+.PP
+\fB\-h\fP, \fB\-\-help\fP[=false]
+ help for edit
+
+
+.SH SEE ALSO
+.PP
+\fBgit\-bug\-comment(1)\fP
diff --git a/doc/man/git-bug-comment.1 b/doc/man/git-bug-comment.1
index 9beb7a06..e4dce2e6 100644
--- a/doc/man/git-bug-comment.1
+++ b/doc/man/git-bug-comment.1
@@ -25,4 +25,4 @@ Display or add comments to a bug.
.SH SEE ALSO
.PP
-\fBgit\-bug(1)\fP, \fBgit\-bug\-comment\-add(1)\fP
+\fBgit\-bug(1)\fP, \fBgit\-bug\-comment\-add(1)\fP, \fBgit\-bug\-comment\-edit(1)\fP
diff --git a/doc/md/git-bug_comment.md b/doc/md/git-bug_comment.md
index 5a79cf6f..14271f69 100644
--- a/doc/md/git-bug_comment.md
+++ b/doc/md/git-bug_comment.md
@@ -20,4 +20,5 @@ git-bug comment [<id>] [flags]
* [git-bug](git-bug.md) - A bug tracker embedded in Git.
* [git-bug comment add](git-bug_comment_add.md) - Add a new comment to a bug.
+* [git-bug comment edit](git-bug_comment_edit.md) - Edit an existing comment on a bug.
diff --git a/doc/md/git-bug_comment_edit.md b/doc/md/git-bug_comment_edit.md
new file mode 100644
index 00000000..0633e479
--- /dev/null
+++ b/doc/md/git-bug_comment_edit.md
@@ -0,0 +1,24 @@
+## git-bug comment edit
+
+Edit an existing comment on a bug.
+
+### Synopsis
+
+Edit an existing comment on a bug.
+
+```
+git-bug comment edit <commentid> [flags]
+```
+
+### Options
+
+```
+ -F, --file string Take the message from the given file. Use - to read the message from the standard input
+ -m, --message string Provide the new message from the command line
+ -h, --help help for edit
+```
+
+### SEE ALSO
+
+* [git-bug comment](git-bug_comment.md) - Display or add comments to a bug.
+
diff --git a/misc/bash_completion/git-bug b/misc/bash_completion/git-bug
index 4152725f..cd9ad1c0 100644
--- a/misc/bash_completion/git-bug
+++ b/misc/bash_completion/git-bug
@@ -654,6 +654,34 @@ _git-bug_comment_add()
noun_aliases=()
}
+_git-bug_comment_edit()
+{
+ last_command="git-bug_comment_edit"
+
+ command_aliases=()
+
+ commands=()
+
+ flags=()
+ two_word_flags=()
+ local_nonpersistent_flags=()
+ flags_with_completion=()
+ flags_completion=()
+
+ flags+=("--file=")
+ two_word_flags+=("--file")
+ two_word_flags+=("-F")
+ local_nonpersistent_flags+=("--file=")
+ flags+=("--message=")
+ two_word_flags+=("--message")
+ two_word_flags+=("-m")
+ local_nonpersistent_flags+=("--message=")
+
+ must_have_one_flag=()
+ must_have_one_noun=()
+ noun_aliases=()
+}
+
_git-bug_comment()
{
last_command="git-bug_comment"
@@ -662,6 +690,7 @@ _git-bug_comment()
commands=()
commands+=("add")
+ commands+=("edit")
flags=()
two_word_flags=()
diff --git a/misc/powershell_completion/git-bug b/misc/powershell_completion/git-bug
index 40831a11..973e4d35 100644
--- a/misc/powershell_completion/git-bug
+++ b/misc/powershell_completion/git-bug
@@ -117,6 +117,7 @@ Register-ArgumentCompleter -Native -CommandName 'git-bug' -ScriptBlock {
}
'git-bug;comment' {
[CompletionResult]::new('add', 'add', [CompletionResultType]::ParameterValue, 'Add a new comment to a bug.')
+ [CompletionResult]::new('edit', 'edit', [CompletionResultType]::ParameterValue, 'Edit an existing comment on a bug.')
break
}
'git-bug;comment;add' {
@@ -126,6 +127,13 @@ Register-ArgumentCompleter -Native -CommandName 'git-bug' -ScriptBlock {
[CompletionResult]::new('--message', 'message', [CompletionResultType]::ParameterName, 'Provide the new message from the command line')
break
}
+ 'git-bug;comment;edit' {
+ [CompletionResult]::new('-F', 'F', [CompletionResultType]::ParameterName, 'Take the message from the given file. Use - to read the message from the standard input')
+ [CompletionResult]::new('--file', 'file', [CompletionResultType]::ParameterName, 'Take the message from the given file. Use - to read the message from the standard input')
+ [CompletionResult]::new('-m', 'm', [CompletionResultType]::ParameterName, 'Provide the new message from the command line')
+ [CompletionResult]::new('--message', 'message', [CompletionResultType]::ParameterName, 'Provide the new message from the command line')
+ break
+ }
'git-bug;deselect' {
break
}
diff --git a/misc/zsh_completion/git-bug b/misc/zsh_completion/git-bug
index fbd52be5..91730e29 100644
--- a/misc/zsh_completion/git-bug
+++ b/misc/zsh_completion/git-bug
@@ -235,6 +235,7 @@ function _git-bug_comment {
cmnds)
commands=(
"add:Add a new comment to a bug."
+ "edit:Edit an existing comment on a bug."
)
_describe "command" commands
;;
@@ -244,6 +245,9 @@ function _git-bug_comment {
add)
_git-bug_comment_add
;;
+ edit)
+ _git-bug_comment_edit
+ ;;
esac
}
@@ -253,6 +257,12 @@ function _git-bug_comment_add {
'(-m --message)'{-m,--message}'[Provide the new message from the command line]:'
}
+function _git-bug_comment_edit {
+ _arguments \
+ '(-F --file)'{-F,--file}'[Take the message from the given file. Use - to read the message from the standard input]:' \
+ '(-m --message)'{-m,--message}'[Provide the new message from the command line]:'
+}
+
function _git-bug_deselect {
_arguments
}
--
GitLab