Change the comment ID to use both bug and comment ID references.

vince created

Add comment edit command
This commit adds the comment edit command, which provides a CLI tool that allows a user to edit a comment.

Change summary

bug/comment.go                     | 46 ++++++++++++++++++++
bug/comment_test.go                | 27 ++++++++++++
bug/op_add_comment.go              |  5 +
bug/op_create.go                   |  5 +
bug/snapshot.go                    |  5 ++
cache/repo_cache_bug.go            | 47 +++++++++++++++++++++
commands/comment.go                |  1 
commands/comment_edit.go           | 71 ++++++++++++++++++++++++++++++++
commands/show.go                   |  3 
doc/man/git-bug-comment-edit.1     | 35 +++++++++++++++
doc/man/git-bug-comment.1          |  2 
doc/md/git-bug_comment.md          |  1 
doc/md/git-bug_comment_edit.md     | 20 +++++++++
misc/bash_completion/git-bug       | 33 ++++++++++++++
misc/powershell_completion/git-bug |  8 +++
15 files changed, 303 insertions(+), 6 deletions(-)

Detailed changes

bug/comment.go 🔗

@@ -1,6 +1,8 @@
 package bug
 
 import (
+	"strings"
+
 	"github.com/dustin/go-humanize"
 
 	"github.com/MichaelMure/git-bug/entity"
@@ -31,6 +33,50 @@ func (c Comment) Id() entity.Id {
 	return c.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.WriteByte(bugId[0])
+			bugId = bugId[1:]
+		} else {
+			id.WriteByte(commentId[0])
+			commentId = commentId[1:]
+		}
+	}
+	return entity.Id(id.String())
+}
+
+func SplitCommentId(prefix string) (bugPrefix string, commentPrefix string) {
+	var bugIdPrefix strings.Builder
+	var commentIdPrefix strings.Builder
+
+	for i, char := range prefix {
+		if compiledCommentIdFormat[i] == 'B' {
+			bugIdPrefix.WriteRune(char)
+		} else {
+			commentIdPrefix.WriteRune(char)
+		}
+	}
+	return bugIdPrefix.String(), commentIdPrefix.String()
+}
+
 // FormatTimeRel format the UnixTime of the comment for human consumption
 func (c Comment) FormatTimeRel() string {
 	return humanize.Time(c.UnixTime.Time())

bug/comment_test.go 🔗

@@ -0,0 +1,27 @@
+package bug
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/MichaelMure/git-bug/entity"
+)
+
+func TestCommentId(t *testing.T) {
+	bugId := entity.Id("abcdefghijklmnopqrstuvwxyz1234__________")
+	opId := entity.Id("ABCDEFGHIJ______________________________")
+	expectedId := entity.Id("aAbBcCdefDghijEklmnFopqrGstuvHwxyzI1234J")
+
+	mergedId := DeriveCommentId(bugId, opId)
+	require.Equal(t, expectedId, mergedId)
+
+	// full length
+	splitBugId, splitCommentId := SplitCommentId(mergedId.String())
+	require.Equal(t, string(bugId[:30]), splitBugId)
+	require.Equal(t, string(opId[:10]), splitCommentId)
+
+	splitBugId, splitCommentId = SplitCommentId(string(expectedId[:6]))
+	require.Equal(t, string(bugId[:3]), splitBugId)
+	require.Equal(t, string(opId[:3]), splitCommentId)
+}

bug/op_add_comment.go 🔗

@@ -36,8 +36,9 @@ func (op *AddCommentOperation) Apply(snapshot *Snapshot) {
 	snapshot.addActor(op.Author)
 	snapshot.addParticipant(op.Author)
 
+	commentId := DeriveCommentId(snapshot.Id(), op.Id())
 	comment := Comment{
-		id:       op.Id(),
+		id:       commentId,
 		Message:  op.Message,
 		Author:   op.Author,
 		Files:    op.Files,
@@ -47,7 +48,7 @@ func (op *AddCommentOperation) Apply(snapshot *Snapshot) {
 	snapshot.Comments = append(snapshot.Comments, comment)
 
 	item := &AddCommentTimelineItem{
-		CommentTimelineItem: NewCommentTimelineItem(op.Id(), comment),
+		CommentTimelineItem: NewCommentTimelineItem(commentId, comment),
 	}
 
 	snapshot.Timeline = append(snapshot.Timeline, item)

bug/op_create.go 🔗

@@ -59,8 +59,9 @@ func (op *CreateOperation) Apply(snapshot *Snapshot) {
 
 	snapshot.Title = op.Title
 
+	commentId := DeriveCommentId(snapshot.Id(), op.Id())
 	comment := Comment{
-		id:       op.Id(),
+		id:       commentId,
 		Message:  op.Message,
 		Author:   op.Author,
 		UnixTime: timestamp.Timestamp(op.UnixTime),
@@ -72,7 +73,7 @@ func (op *CreateOperation) Apply(snapshot *Snapshot) {
 
 	snapshot.Timeline = []TimelineItem{
 		&CreateTimelineItem{
-			CommentTimelineItem: NewCommentTimelineItem(op.Id(), comment),
+			CommentTimelineItem: NewCommentTimelineItem(commentId, comment),
 		},
 	}
 }

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
 }
 

cache/repo_cache_bug.go 🔗

@@ -261,6 +261,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, error) {
 	c.muBug.RLock()

commands/comment.go 🔗

@@ -22,6 +22,7 @@ func newCommentCommand() *cobra.Command {
 	}
 
 	cmd.AddCommand(newCommentAddCommand())
+	cmd.AddCommand(newCommentEditCommand())
 
 	return cmd
 }

commands/comment_edit.go 🔗

@@ -0,0 +1,71 @@
+package commands
+
+import (
+	"github.com/spf13/cobra"
+
+	"github.com/MichaelMure/git-bug/input"
+)
+
+type commentEditOptions struct {
+	messageFile string
+	message     string
+}
+
+func newCommentEditCommand() *cobra.Command {
+	env := newEnv()
+	options := commentEditOptions{}
+
+	cmd := &cobra.Command{
+		Use:      "edit <commentid>",
+		Short:    "Edit an existing comment on a bug.",
+		Args:     cobra.ExactArgs(1),
+		PreRunE:  loadBackendEnsureUser(env),
+		PostRunE: closeBackend(env),
+		RunE: func(cmd *cobra.Command, args []string) error {
+			return runCommentEdit(env, options, args)
+		},
+	}
+
+	flags := cmd.Flags()
+	flags.SortFlags = false
+
+	flags.StringVarP(&options.messageFile, "file", "F", "",
+		"Take the message from the given file. Use - to read the message from the standard input")
+
+	flags.StringVarP(&options.message, "message", "m", "",
+		"Provide the new message from the command line")
+
+	return cmd
+}
+
+func runCommentEdit(env *Env, opts commentEditOptions, args []string) error {
+	b, commentId, err := env.backend.ResolveComment(args[0])
+	if err != nil {
+		return err
+	}
+
+	if opts.messageFile != "" && opts.message == "" {
+		opts.message, err = input.BugCommentFileInput(opts.messageFile)
+		if err != nil {
+			return err
+		}
+	}
+
+	if opts.messageFile == "" && opts.message == "" {
+		opts.message, err = input.BugCommentEditorInput(env.backend, "")
+		if err == input.ErrEmptyMessage {
+			env.err.Println("Empty message, aborting.")
+			return nil
+		}
+		if err != nil {
+			return err
+		}
+	}
+
+	_, err = b.EditComment(commentId, opts.message)
+	if err != nil {
+		return err
+	}
+
+	return b.Commit()
+}

commands/show.go 🔗

@@ -158,8 +158,9 @@ func showDefaultFormatter(env *Env, snapshot *bug.Snapshot) error {
 
 	for i, comment := range snapshot.Comments {
 		var message string
-		env.out.Printf("%s#%d %s <%s>\n\n",
+		env.out.Printf("%s%s #%d %s <%s>\n\n",
 			indent,
+			comment.Id().Human(),
 			i,
 			comment.Author.DisplayName(),
 			comment.Author.Email(),

doc/man/git-bug-comment-edit.1 🔗

@@ -0,0 +1,35 @@
+.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

doc/man/git-bug-comment.1 🔗

@@ -24,4 +24,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

doc/md/git-bug_comment.md 🔗

@@ -16,4 +16,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.
 

doc/md/git-bug_comment_edit.md 🔗

@@ -0,0 +1,20 @@
+## git-bug comment edit
+
+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.
+

misc/bash_completion/git-bug 🔗

@@ -722,6 +722,38 @@ _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")
+    local_nonpersistent_flags+=("--file=")
+    local_nonpersistent_flags+=("-F")
+    flags+=("--message=")
+    two_word_flags+=("--message")
+    two_word_flags+=("-m")
+    local_nonpersistent_flags+=("--message")
+    local_nonpersistent_flags+=("--message=")
+    local_nonpersistent_flags+=("-m")
+
+    must_have_one_flag=()
+    must_have_one_noun=()
+    noun_aliases=()
+}
+
 _git-bug_comment()
 {
     last_command="git-bug_comment"
@@ -730,6 +762,7 @@ _git-bug_comment()
 
     commands=()
     commands+=("add")
+    commands+=("edit")
 
     flags=()
     two_word_flags=()

misc/powershell_completion/git-bug 🔗

@@ -118,6 +118,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' {
@@ -127,6 +128,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
         }