feat: add heading search to ask and todo cmds - #49
nimaaskarian wants to merge 4 commits into
Conversation
|
@nimaaskarian Thanks for the pull request! OK, I see what you're trying to do here, and I guess I can see a use case where some people might want this depending on how they've structured their habits (as you say, bracketed). May I just ask you to write some proper tests for the feature before I merge? Right now it looks like you just updated the existing tests to have it pass with search and todo but do not have tests for testing if you get an expected heading search etc for a specific input and such (not trying to be difficult, just trying to avoid regressions and such.). Also, do you mind me asking how you're personally using it? (just curious, and since I group my habits into daily/weekly(ish)/monthly+ headings it's not necessarily applicable to my use case.). =] |
|
i'm doing fine! as fine as i can be given the circumstances in my country. what about you? as for the tests, yeah i've done a classical move of a lazy software developer. i'll write some tests for it later today. i've heard about the method i'm using in a Huberman podcast episode and it works pretty fine. it increased my habit scores in the past month vs the month before (yes, i did this commit a month ago on my own fork but the internet in my country was completely shut off) i personally bracket my habits into 3 phases; 0 to 8 hours after waking, 9 to 15 hours after waking and 16 to 20 hours after waking. the hour i've woke up is extracted from my wake up tracking habit (which the amount is time of day in hours i've woke up), and defaults to
so i track per-task score with a simple python script, and move around my tasks in these three headings after a month or so. as Huberman claims, there's proof in literature that moving around the time of the task is actually helpful for forming the habit. |
There was a problem hiding this comment.
Pull request overview
Adds an optional --heading filter to the ask and todo CLI flows so users can narrow displayed/asked todos to habits whose Heading contains a given substring.
Changes:
- Extend
ui.GetTodosand related call sites to accept asearchHeadingfilter. - Add
--headingflag (with shell completion) toaskandtodo; add--no-printtotodoto suppress the “no todos” message. - Update and add tests to cover heading-filtered todo output and the new “no todos” printing behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
internal/ui/display.go |
Adds searchHeading filtering to GetTodos and threads heading into todo rendering. |
internal/ui/input.go |
Threads heading into the interactive ask flow by passing it to GetTodos. |
cmd/ask.go |
Adds --heading flag + completion; passes heading into AskHabits. |
cmd/todo.go |
Adds --heading and --no-print flags; passes both into ShowTodos. |
test/ui_test.go |
Updates tests for new signatures and adds coverage for heading-filtering and “no todos” output suppression. |
test/integration_test.go |
Updates GetTodos call sites for the new parameter. |
test/harsh_test.go |
Updates GetTodos call site for the new parameter. |
Comments suppressed due to low confidence (1)
internal/ui/display.go:265
GetTodosapplies thesearchHeadingfilter only in thedaysBack != 0branch. WhendaysBack == 0(onboarding path), heading filtering is ignored, so--headingwill still return all habits. Apply the same heading filtering in thedaysBack == 0branch (and ideally preserve file order by iteratinghabits, not thedayHabitsmap).
func GetTodos(habits []*storage.Habit, entries *storage.Entries, to civil.Date, daysBack int, searchHeading string) map[string][]string {
tasksUndone := map[string][]string{}
dayHabits := map[string]bool{}
from := to.AddDays(-daysBack)
noFirstRecord := civil.Date{Year: 0, Month: 0, Day: 0}
// Put in conditional for onboarding starting at 0 days or normal lookback
if daysBack == 0 {
for _, habit := range habits {
dayHabits[habit.Name] = true
}
for habit := range dayHabits {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func headingCompletion(cmd *cobra.Command, args []string, toComplete string) ([]cobra.Completion, cobra.ShellCompDirective) { | ||
| out := []cobra.Completion{} | ||
| for _, habit := range harsh.GetHabits() { | ||
| if strings.Contains(habit.Heading, toComplete) { | ||
| out = append(out, habit.Heading) | ||
| } | ||
| } | ||
| return out, cobra.ShellCompDirectiveNoFileComp |
There was a problem hiding this comment.
headingCompletion iterates over harsh.GetHabits() directly. If the global harsh instance hasn't been initialized yet (common during shell completion), this can panic with a nil pointer dereference. Use getHarsh() inside the completion function (as done in askCmdValidArgs) and iterate over that instance's habits.
| now := civil.DateOf(time.Now()) | ||
| for day.Before(now) { | ||
| day = day.AddDays(1) | ||
| (*entries)[storage.DailyHabit{Day: day, Habit: "Test1"}] = storage.Outcome{Result: "y"} | ||
| } |
There was a problem hiding this comment.
TestDoneHabits depends on the current date (time.Now()) and loops from a fixed start date (2025-01-01) up to today. This makes the test non-deterministic and will steadily get slower over time. Use a fixed now date for the test (and/or set FirstRecord relative to that fixed date) so the loop bounds and expectations are stable.
|
@nimaaskarian Hey... so ran copilot over the PR. ANy changes you want to make due to the suggestions/issues raised? (esp ask.go since it would raise an exception... ). |
|
Thanks @nimaaskarian, this sounds like a great improvement to me. @wakatara actually I do see the benefits even when you have your habits simply grouped in daily/weekly(ish)/monthly+ headings since you might not want |
|
@wakatara hey... sorry for this PR not being active for a while. the situation in my country went nuts a bit and the internet got shut down for a few months. i'll checkout the copilot exceptions and try to fix the issues raised |
|
Hey sir! Hope you are doing "alright" (considering). And absolutely no need for apologies considering the circumstances. Take your time and answer whenever you have a chance. Keep safe. Welcome back. |
using
--headingin subcommandsaskandtodo, you can now show tasks that their heading includes a sub string.this is helpful with task bracketing systems. e.g. putting harder tasks in a bracket in which you just woke up and chances of you doing the habit is higher.