refactor!: rework tag parsing

* refactor(tag-parsing)!: remove tags-as-tasks flag

To determine if tags should be treated as flags should happen based on the value of the corresponding regex. If the regex is empty (""), the condition should be false, otherwise, we should treat tags as tasks.

BREAKING CHANGE: This refactoring corrects a wrong assumption in the timewarrior source, that used tags-as-tasks flag. From now on, the timewarrior client will split entries as the other sources if the split condition matches.

* test: use `ElementsMatch` over `Equal`
* refactor: move tags-as-tasks in-house to FetchOpts
* docs: fix comment wording
* refactor: unify how regex is checked
* chore(changelog): update changelog

CHANGELOG.md

@@ -2,6 +2,22 @@
 
 All notable changes to this project will be documented in this file.
 
+## [unreleased]
+
+**Documentation**
+
+- Fix comment wording ([cb3b3bb](https://github.com/gabor-boros/minutes/commit/cb3b3bb9763bdb6c68d5e93d5f7f16de0605abfe))
+
+**Refactor**
+
+- Remove tags-as-tasks flag ([8ea8769](https://github.com/gabor-boros/minutes/commit/8ea87697a14c59070d149bcca0823c2cc69228c7))
+- Move tags-as-tasks in-house to FetchOpts ([6caa95a](https://github.com/gabor-boros/minutes/commit/6caa95a91e4b0573057868563d19570e90383659))
+- Unify how regex is checked ([0c393c5](https://github.com/gabor-boros/minutes/commit/0c393c586ae1af8298f4d207f7459776efb24bfc))
+
+**Testing**
+
+- Use `ElementsMatch` over `Equal` ([841e0df](https://github.com/gabor-boros/minutes/commit/841e0df9a0ccd4a6b6067b7cff494b89f4c7cbe5))
+
 ## [0.2.3] - 2021-11-08
 
 **Build**

README.md

@@ -86,7 +86,6 @@ Flags:
       --start string                           set the start date (defaults to 00:00:00)
       --table-hide-column strings              hide table column [summary project client start end]
       --table-sort-by strings                  sort table by column [task summary project client start end billable unbillable] (default [start,project,task,summary])
-      --tags-as-tasks                          treat tags matching the value of tags-as-tasks-regex as tasks
       --tags-as-tasks-regex string             regex of the task pattern
   -t, --target string                          set the target of the sync [tempo]
       --target-user string                     set the source user ID
@@ -130,7 +129,7 @@ $ minutes --date-format "2006-01-02" --start "2021-10-07" --end "2021-10-08"
 
 ```shell
 # Specify how a tag should look like to be considered as a task
-$ minutes --tags-as-tasks --tags-as-tasks-regex '[A-Z]{2,7}-\d{1,6}'
+$ minutes --tags-as-tasks-regex '[A-Z]{2,7}-\d{1,6}'
 ```
 
 #### Minute based rounding
@@ -161,7 +160,6 @@ tempo-username = "<jira username>"
 tempo-password = "<jira password>"
 
 # General config
-tags-as-tasks = true
 tags-as-tasks-regex = '[A-Z]{2,7}-\d{1,6}'
 round-to-closest-minute = true
 force-billed-duration = true

cmd/root.go

@@ -127,7 +127,6 @@ func initCommonFlags() {
 	rootCmd.Flags().StringSliceP("table-sort-by", "", []string{utils.ColumnStart, utils.ColumnProject, utils.ColumnTask, utils.ColumnSummary}, fmt.Sprintf("sort table by column %v", utils.Columns))
 	rootCmd.Flags().StringSliceP("table-hide-column", "", []string{}, fmt.Sprintf("hide table column %v", utils.HideableColumns))
 
-	rootCmd.Flags().BoolP("tags-as-tasks", "", false, "treat tags matching the value of tags-as-tasks-regex as tasks")
 	rootCmd.Flags().StringP("tags-as-tasks-regex", "", "", "regex of the task pattern")
 
 	rootCmd.Flags().BoolP("round-to-closest-minute", "", false, "round time to closest minute")
@@ -196,16 +195,9 @@ func validateFlags() {
 		cobra.CheckErr(fmt.Sprintf("\"%s\" is not part of the supported targets %v\n", target, targets))
 	}
 
-	if viper.GetBool("tags-as-tasks") {
-		tasksAsTagsRegex := viper.GetString("tags-as-tasks-regex")
-
-		if tasksAsTagsRegex == "" {
-			cobra.CheckErr("tags-as-tasks-regex cannot be empty if tags-as-tasks is set")
-		}
-
-		_, err = regexp.Compile(tasksAsTagsRegex)
-		cobra.CheckErr(err)
-	}
+	tagsAsTasksRegex := viper.GetString("tags-as-tasks-regex")
+	_, err = regexp.Compile(tagsAsTasksRegex)
+	cobra.CheckErr(err)
 
 	for _, sortBy := range viper.GetStringSlice("table-sort-by") {
 		column := sortBy
@@ -252,18 +244,12 @@ func validateFlags() {
 }
 
 func getFetcher() (client.Fetcher, error) {
-	tagsAsTasksRegex, err := regexp.Compile(viper.GetString("tags-as-tasks-regex"))
-	if err != nil {
-		return nil, err
-	}
 
 	switch viper.GetString("source") {
 	case "clockify":
 		return clockify.NewFetcher(&clockify.ClientOpts{
 			BaseClientOpts: client.BaseClientOpts{
-				TagsAsTasks:      viper.GetBool("tags-as-tasks"),
-				TagsAsTasksRegex: tagsAsTasksRegex,
-				Timeout:          client.DefaultRequestTimeout,
+				Timeout: client.DefaultRequestTimeout,
 			},
 			TokenAuth: client.TokenAuth{
 				Header: "X-Api-Key",
@@ -275,9 +261,7 @@ func getFetcher() (client.Fetcher, error) {
 	case "harvest":
 		return harvest.NewFetcher(&harvest.ClientOpts{
 			BaseClientOpts: client.BaseClientOpts{
-				TagsAsTasks:      viper.GetBool("tags-as-tasks"),
-				TagsAsTasksRegex: tagsAsTasksRegex,
-				Timeout:          client.DefaultRequestTimeout,
+				Timeout: client.DefaultRequestTimeout,
 			},
 			TokenAuth: client.TokenAuth{
 				TokenName: "Bearer",
@@ -289,9 +273,7 @@ func getFetcher() (client.Fetcher, error) {
 	case "tempo":
 		return tempo.NewFetcher(&tempo.ClientOpts{
 			BaseClientOpts: client.BaseClientOpts{
-				TagsAsTasks:      viper.GetBool("tags-as-tasks"),
-				TagsAsTasksRegex: tagsAsTasksRegex,
-				Timeout:          client.DefaultRequestTimeout,
+				Timeout: client.DefaultRequestTimeout,
 			},
 			BasicAuth: client.BasicAuth{
 				Username: viper.GetString("tempo-username"),
@@ -302,9 +284,7 @@ func getFetcher() (client.Fetcher, error) {
 	case "timewarrior":
 		return timewarrior.NewFetcher(&timewarrior.ClientOpts{
 			BaseClientOpts: client.BaseClientOpts{
-				TagsAsTasks:      viper.GetBool("tags-as-tasks"),
-				TagsAsTasksRegex: tagsAsTasksRegex,
-				Timeout:          client.DefaultRequestTimeout,
+				Timeout: client.DefaultRequestTimeout,
 			},
 			CLIClient: client.CLIClient{
 				Command:            viper.GetString("timewarrior-command"),
@@ -318,9 +298,7 @@ func getFetcher() (client.Fetcher, error) {
 	case "toggl":
 		return toggl.NewFetcher(&toggl.ClientOpts{
 			BaseClientOpts: client.BaseClientOpts{
-				TagsAsTasks:      viper.GetBool("tags-as-tasks"),
-				TagsAsTasksRegex: tagsAsTasksRegex,
-				Timeout:          client.DefaultRequestTimeout,
+				Timeout: client.DefaultRequestTimeout,
 			},
 			BasicAuth: client.BasicAuth{
 				Username: viper.GetString("toggl-api-key"),
@@ -339,8 +317,7 @@ func getUploader() (client.Uploader, error) {
 	case "tempo":
 		return tempo.NewUploader(&tempo.ClientOpts{
 			BaseClientOpts: client.BaseClientOpts{
-				TagsAsTasks: viper.GetBool("tags-as-tasks"),
-				Timeout:     client.DefaultRequestTimeout,
+				Timeout: client.DefaultRequestTimeout,
 			},
 			BasicAuth: client.BasicAuth{
 				Username: viper.GetString("tempo-username"),
@@ -387,10 +364,14 @@ func runRootCmd(_ *cobra.Command, _ []string) {
 	uploader, err := getUploader()
 	cobra.CheckErr(err)
 
+	tagsAsTasksRegex, err := regexp.Compile(viper.GetString("tags-as-tasks-regex"))
+	cobra.CheckErr(err)
+
 	entries, err := fetcher.FetchEntries(context.Background(), &client.FetchOpts{
-		End:   end,
-		Start: start,
-		User:  viper.GetString("source-user"),
+		End:              end,
+		Start:            start,
+		User:             viper.GetString("source-user"),
+		TagsAsTasksRegex: tagsAsTasksRegex,
 	})
 	cobra.CheckErr(err)
 

internal/pkg/client/client.go

@@ -12,7 +12,6 @@ import (
 	netURL "net/url"
 	"os/exec"
 	"reflect"
-	"regexp"
 	"strconv"
 	"time"
 
@@ -40,18 +39,6 @@ var (
 // When a client needs other options as well, it composes a new set of options
 // using BaseClientOpts.
 type BaseClientOpts struct {
-	// TagsAsTasks defines to use tag names to determine the task.
-	// Using TagsAsTasks can be useful if the user's workflow involves
-	// splitting activity across multiple tasks, or when the user has no option
-	// to set multiple tasks for a single activity.
-	//
-	// This option must be used in conjunction with TagsAsTasksRegex option.
-	TagsAsTasks bool
-	// TagsAsTasksRegex sets the regular expression used for extracting tasks
-	// from the list of tags.
-	//
-	// This option must be used in conjunction with TagsAsTasks option.
-	TagsAsTasksRegex *regexp.Regexp
 	// Timeout sets the timeout for the client to execute a request.
 	// In the case of HTTP clients, the timeout is applied on the HTTP request,
 	// while in the case of CLI based clients it will be applied on the command
@@ -252,7 +239,7 @@ func (c *HTTPClient) PaginatedFetch(ctx context.Context, opts *PaginatedFetchOpt
 			break
 		}
 
-		parsedEntries, err := opts.ParseFunc(rawEntries)
+		parsedEntries, err := opts.ParseFunc(rawEntries, opts.BaseFetchOpts)
 		if err != nil {
 			return nil, fmt.Errorf("%v: %v", ErrFetchEntries, err)
 		}

internal/pkg/client/clockify/clockify.go

@@ -70,7 +70,7 @@ type clockifyClient struct {
 	workspace     string
 }
 
-func (c *clockifyClient) parseEntries(rawEntries interface{}) (worklog.Entries, error) {
+func (c *clockifyClient) parseEntries(rawEntries interface{}, opts *client.FetchOpts) (worklog.Entries, error) {
 	var entries worklog.Entries
 
 	fetchedEntries, ok := rawEntries.([]FetchEntry)
@@ -107,8 +107,8 @@ func (c *clockifyClient) parseEntries(rawEntries interface{}) (worklog.Entries,
 			UnbillableDuration: unbillableDuration,
 		}
 
-		if c.TagsAsTasks && len(entry.Tags) > 0 {
-			pageEntries := worklogEntry.SplitByTagsAsTasks(entry.Description, c.TagsAsTasksRegex, entry.Tags)
+		if utils.IsRegexSet(opts.TagsAsTasksRegex) && len(entry.Tags) > 0 {
+			pageEntries := worklogEntry.SplitByTagsAsTasks(entry.Description, opts.TagsAsTasksRegex, entry.Tags)
 			entries = append(entries, pageEntries...)
 		} else {
 			entries = append(entries, worklogEntry)
@@ -151,6 +151,7 @@ func (c *clockifyClient) FetchEntries(ctx context.Context, opts *client.FetchOpt
 	}
 
 	return c.PaginatedFetch(ctx, &client.PaginatedFetchOpts{
+		BaseFetchOpts: opts,
 		URL:           fetchURL,
 		PageSizeParam: "page-size",
 		FetchFunc:     c.fetchEntries,

internal/pkg/client/clockify/clockify_test.go

@@ -212,7 +212,7 @@ func TestClockifyClient_FetchEntries(t *testing.T) {
 	require.ElementsMatch(t, expectedEntries, entries, "fetched entries are not matching")
 }
 
-func TestClockifyClient_FetchEntries_TasksAsTags(t *testing.T) {
+func TestClockifyClient_FetchEntries_TagsAsTasks(t *testing.T) {
 	start := time.Date(2021, 10, 2, 0, 0, 0, 0, time.UTC)
 	end := time.Date(2021, 10, 2, 23, 59, 59, 0, time.UTC)
 	remainingCalls := 1
@@ -357,9 +357,7 @@ func TestClockifyClient_FetchEntries_TasksAsTags(t *testing.T) {
 
 	clockifyClient, err := clockify.NewFetcher(&clockify.ClientOpts{
 		BaseClientOpts: client.BaseClientOpts{
-			TagsAsTasks:      true,
-			TagsAsTasksRegex: regexp.MustCompile(`^TASK-\d+$`),
-			Timeout:          client.DefaultRequestTimeout,
+			Timeout: client.DefaultRequestTimeout,
 		},
 		TokenAuth: client.TokenAuth{
 			Header: "X-Api-Key",
@@ -372,9 +370,10 @@ func TestClockifyClient_FetchEntries_TasksAsTags(t *testing.T) {
 	require.Nil(t, err)
 
 	entries, err := clockifyClient.FetchEntries(context.Background(), &client.FetchOpts{
-		User:  "steve-rogers",
-		Start: start,
-		End:   end,
+		User:             "steve-rogers",
+		Start:            start,
+		End:              end,
+		TagsAsTasksRegex: regexp.MustCompile(`^TASK-\d+$`),
 	})
 
 	require.Nil(t, err, "cannot fetch entries")

internal/pkg/client/fetcher.go

@@ -3,6 +3,7 @@ package client
 import (
 	"context"
 	"errors"
+	"regexp"
 	"time"
 
 	"github.com/gabor-boros/minutes/internal/pkg/worklog"
@@ -31,6 +32,10 @@ type FetchOpts struct {
 	User  string
 	Start time.Time
 	End   time.Time
+
+	// TagsAsTasksRegex sets the regular expression used for extracting tasks
+	// from the list of tags.
+	TagsAsTasksRegex *regexp.Regexp
 }
 
 // Fetcher specifies the functions used to fetch worklog entries.
@@ -47,9 +52,11 @@ type PaginatedFetchResponse struct {
 }
 
 type PaginatedFetchFunc = func(context.Context, string) (interface{}, *PaginatedFetchResponse, error)
-type PaginatedParseFunc = func(interface{}) (worklog.Entries, error)
+type PaginatedParseFunc = func(interface{}, *FetchOpts) (worklog.Entries, error)
 
 type PaginatedFetchOpts struct {
+	BaseFetchOpts *FetchOpts
+
 	URL           string
 	PageSize      int
 	PageSizeParam string

internal/pkg/client/harvest/harvest.go

@@ -82,7 +82,7 @@ type harvestClient struct {
 	account       int
 }
 
-func (c *harvestClient) parseEntries(rawEntries interface{}) (worklog.Entries, error) {
+func (c *harvestClient) parseEntries(rawEntries interface{}, _ *client.FetchOpts) (worklog.Entries, error) {
 	var entries worklog.Entries
 
 	fetchedEntries, ok := rawEntries.([]FetchEntry)

internal/pkg/client/timewarrior/timewarrior.go

@@ -43,7 +43,7 @@ type timewarriorClient struct {
 	unbillableTag   string
 }
 
-func (c *timewarriorClient) parseEntry(entry FetchEntry) (worklog.Entries, error) {
+func (c *timewarriorClient) parseEntry(entry FetchEntry, opts *client.FetchOpts) (worklog.Entries, error) {
 	var entries worklog.Entries
 
 	startDate, err := time.ParseInLocation(utils.DateFormatRFC3339Compact.String(), entry.Start, time.Local)
@@ -68,17 +68,17 @@ func (c *timewarriorClient) parseEntry(entry FetchEntry) (worklog.Entries, error
 		if tag == c.unbillableTag {
 			worklogEntry.UnbillableDuration = worklogEntry.BillableDuration
 			worklogEntry.BillableDuration = 0
-		} else if c.clientTagRegex.String() != "" && c.clientTagRegex.MatchString(tag) {
+		} else if utils.IsRegexSet(c.clientTagRegex) && c.clientTagRegex.MatchString(tag) {
 			worklogEntry.Client = worklog.IDNameField{
 				ID:   tag,
 				Name: tag,
 			}
-		} else if c.projectTagRegex.String() != "" && c.projectTagRegex.MatchString(tag) {
+		} else if utils.IsRegexSet(c.projectTagRegex) && c.projectTagRegex.MatchString(tag) {
 			worklogEntry.Project = worklog.IDNameField{
 				ID:   tag,
 				Name: tag,
 			}
-		} else if c.TagsAsTasksRegex.String() != "" && c.TagsAsTasksRegex.MatchString(tag) {
+		} else if utils.IsRegexSet(opts.TagsAsTasksRegex) && opts.TagsAsTasksRegex.MatchString(tag) {
 			worklogEntry.Task = worklog.IDNameField{
 				ID:   tag,
 				Name: tag,
@@ -94,7 +94,7 @@ func (c *timewarriorClient) parseEntry(entry FetchEntry) (worklog.Entries, error
 		}
 	}
 
-	if c.TagsAsTasks && len(entry.Tags) > 0 {
+	if utils.IsRegexSet(opts.TagsAsTasksRegex) && len(entry.Tags) > 0 {
 		var tags []worklog.IDNameField
 		for _, tag := range entry.Tags {
 			tags = append(tags, worklog.IDNameField{
@@ -103,7 +103,7 @@ func (c *timewarriorClient) parseEntry(entry FetchEntry) (worklog.Entries, error
 			})
 		}
 
-		splitEntries := worklogEntry.SplitByTagsAsTasks(worklogEntry.Summary, c.TagsAsTasksRegex, tags)
+		splitEntries := worklogEntry.SplitByTagsAsTasks(worklogEntry.Summary, opts.TagsAsTasksRegex, tags)
 		entries = append(entries, splitEntries...)
 	} else {
 		entries = append(entries, worklogEntry)
@@ -148,7 +148,7 @@ func (c *timewarriorClient) FetchEntries(ctx context.Context, opts *client.Fetch
 
 	var entries worklog.Entries
 	for _, entry := range fetchedEntries {
-		parsedEntries, err := c.parseEntry(entry)
+		parsedEntries, err := c.parseEntry(entry, opts)
 		if err != nil {
 			return nil, fmt.Errorf("%v: %v", client.ErrFetchEntries, err)
 		}