Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 0 additions & 24 deletions cmd/break.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
package cmd

import (
"fmt"
"strconv"
"time"

"github.com/justincampbell/go-countdown"
"github.com/justincampbell/go-countdown/format"
"github.com/open-pomodoro/openpomodoro-cli/hook"
"github.com/spf13/cobra"
)
Expand Down Expand Up @@ -42,21 +36,3 @@ func breakCmd(cmd *cobra.Command, args []string) error {

return hook.Run(client, "stop")
}

func wait(d time.Duration) error {
err := countdown.For(d, time.Second).Do(func(c *countdown.Countdown) error {
fmt.Printf("\r%s", format.MinSec(c.Remaining()))
return nil
})

fmt.Println()

return err
}

func parseDurationMinutes(s string) (time.Duration, error) {
if _, err := strconv.Atoi(s); err == nil {
s = fmt.Sprintf("%sm", s)
}
return time.ParseDuration(s)
}
35 changes: 34 additions & 1 deletion cmd/finish.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,26 @@ package cmd

import (
"fmt"
"strings"
"time"

"github.com/open-pomodoro/openpomodoro-cli/format"
"github.com/open-pomodoro/openpomodoro-cli/hook"
"github.com/spf13/cobra"
)

var breakFlag string

func init() {
command := &cobra.Command{
Use: "finish",
Short: "Finish the current Pomodoro",
RunE: finishCmd,
}

command.Flags().StringVarP(&breakFlag, "break", "b", "", "take a break after finishing (duration in minutes)")
command.Flags().Lookup("break").NoOptDefVal = " "
Copy link

Copilot AI Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a space character as the NoOptDefVal is unclear and could be confusing. Consider using an empty string or a more explicit default value like "default" to make the intent clearer.

Suggested change
command.Flags().Lookup("break").NoOptDefVal = " "
command.Flags().Lookup("break").NoOptDefVal = ""

Copilot uses AI. Check for mistakes.

RootCmd.AddCommand(command)
}

Expand All @@ -32,5 +38,32 @@ func finishCmd(cmd *cobra.Command, args []string) error {
return err
}

return client.Finish()
if err := client.Finish(); err != nil {
return err
}

if cmd.Flags().Changed("break") {
breakDuration := settings.DefaultBreakDuration

trimmedFlag := strings.TrimSpace(breakFlag)
if trimmedFlag != "" {
Comment on lines +48 to +49
Copy link

Copilot AI Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic for handling the space character NoOptDefVal is indirect and hard to follow. Since you're trimming spaces and checking for emptiness, the space character from NoOptDefVal will always result in using the default duration, making this approach unnecessarily complex.

Copilot uses AI. Check for mistakes.
var err error
breakDuration, err = parseDurationMinutes(trimmedFlag)
if err != nil {
return err
}
}

if err := hook.Run(client, "break"); err != nil {
return err
}

if err := wait(breakDuration); err != nil {
return err
}

return hook.Run(client, "stop")
}

return nil
}
30 changes: 30 additions & 0 deletions cmd/util.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package cmd

import (
"fmt"
"strconv"
"time"

"github.com/justincampbell/go-countdown"
"github.com/justincampbell/go-countdown/format"
)

// wait displays a countdown timer for the specified duration
func wait(d time.Duration) error {
err := countdown.For(d, time.Second).Do(func(c *countdown.Countdown) error {
fmt.Printf("\r%s", format.MinSec(c.Remaining()))
return nil
})

fmt.Println()

return err
}

// parseDurationMinutes parses a duration string, defaulting to minutes if no unit is specified
func parseDurationMinutes(s string) (time.Duration, error) {
if _, err := strconv.Atoi(s); err == nil {
s = fmt.Sprintf("%sm", s)
}
return time.ParseDuration(s)
}
66 changes: 66 additions & 0 deletions cmd/util_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package cmd

import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestParseDurationMinutes(t *testing.T) {
tests := []struct {
name string
input string
expected time.Duration
wantErr bool
}{
{
name: "integer defaults to minutes",
input: "5",
expected: 5 * time.Minute,
},
{
name: "explicit minutes",
input: "10m",
expected: 10 * time.Minute,
},
{
name: "seconds",
input: "30s",
expected: 30 * time.Second,
},
{
name: "hours",
input: "1h",
expected: 1 * time.Hour,
},
{
name: "complex duration",
input: "1h30m",
expected: 90 * time.Minute,
},
{
name: "invalid duration",
input: "invalid",
wantErr: true,
},
{
name: "zero",
input: "0",
expected: 0,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := parseDurationMinutes(tt.input)
if tt.wantErr {
require.Error(t, err)
} else {
require.NoError(t, err)
assert.Equal(t, tt.expected, got)
}
})
}
}
16 changes: 16 additions & 0 deletions test/finish.bats
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,19 @@ load test_helper
[ "$status" -eq 0 ]
assert_file_empty "current"
}

@test "finish --break flag is accepted" {
create_hook "break" 'exit 1'

pomodoro start "Task" --ago 1m
run pomodoro finish --break
[ "$status" -ne 0 ]
}

@test "finish --break with custom duration is accepted" {
create_hook "break" 'exit 1'

pomodoro start "Task" --ago 1m
run pomodoro finish --break 5
[ "$status" -ne 0 ]
}