Skip to content

Commit

Permalink
Fix up undoStack.RunIfError
Browse files Browse the repository at this point in the history
First, with Go 1.14, the use of `%w` in a Printf context other than Errorf
causes a vet error.  `%w` is a special trigger to Errorf to create a
"wrapped" error.  What you'd want to do instead is either `%s` or `%v`.
However, given the nature of error strings, the behavior of simply
concatenating them together to `w` seems inappropriate.  If anything,
they should at least occur on seperate lines, so Fprintln seems more
appropriate.

Second, RunIfError's comment and invocation in Exec made apparent
another bug.  As written, the undo stack would never actually execute.
As directed, if you were to:

	defer undoStack.RunIfError(out, err)

...every value is evaluated at the point of `defer`, but the execution of
the function is what is actually deferred.  `err` will be `nil` there,
and thus the undo stack is never actually executed.

This simple fix there is to actually defer a closure.  When an error is
returned, it will then be observable from within the closure.
  • Loading branch information
jeddenlea committed Mar 25, 2020
1 parent 3b6f710 commit fe774da
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
4 changes: 2 additions & 2 deletions pkg/common/undo.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,15 @@ func (s *UndoStack) Len() int {
// error, it gets logged to the provided writer. Should be deferrerd, such as:
//
// undoStack := common.NewUndoStack()
// defer undoStack.RunIfError(w, err)
// defer func() { undoStack.RunIfError(w, err) }()
//
func (s *UndoStack) RunIfError(w io.Writer, err error) {
if err == nil {
return
}
for i := len(s.states) - 1; i >= 0; i-- {
if err := s.states[i](); err != nil {
fmt.Fprintf(w, "%w", err)
fmt.Fprintln(w, err)
}
}
}
2 changes: 1 addition & 1 deletion pkg/compute/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func (c *InitCommand) Exec(in io.Reader, out io.Writer) (err error) {
}()

undoStack := common.NewUndoStack()
defer undoStack.RunIfError(out, err)
defer func() { undoStack.RunIfError(out, err) }()

if c.path == "" {
fmt.Fprintf(progress, "--path not specified, using current directory\n")
Expand Down

0 comments on commit fe774da

Please sign in to comment.