-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Month calendars #207
Month calendars #207
Conversation
WalkthroughThe pull request introduces several updates to the Changes
Assessment against linked issues
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
Guppi.Console/Skills/CalendarSkill.cs (3)
47-52
: LGTM! Consider enhancing the command description.The command registration follows the established pattern and correctly implements the markdown option. Consider making the description more descriptive, e.g., "Displays the current month's calendar with optional markdown format for Obsidian".
-var month = new Command("month", "Displays this month's calendar") { markdown }; +var month = new Command("month", "Displays the current month's calendar with optional markdown format for Obsidian") { markdown };
310-331
: Enhance markdown format for better Obsidian integration.The markdown implementation is good but could be improved for better organization in Obsidian.
Consider these enhancements:
private static async Task MonthMarkdown() { (DateOnly start, DateOnly end) = GetMonthRange(); StringBuilder cal = new(); + cal.AppendLine($"# {start:MMMM yyyy}"); + cal.AppendLine(); - cal.AppendLine("| Day | Date | Habits | Notes |"); - cal.AppendLine("| --- | ---- | ------ | ----- |"); + cal.AppendLine("| Day | Date | Tasks |"); + cal.AppendLine("| --- | ---- | ----- |"); for (var day = start; day <= end; day = day.AddDays(1)) { - cal.AppendLine($"| **{day:ddd}** | [[{day:yyyy-MM-dd}]] | | |"); + cal.AppendLine($"| **{day:ddd}** | [[{day:yyyy-MM-dd}]] | |"); } // ... rest of the method }
333-339
: Improve testability and documentation of the helper method.Consider these improvements:
+ /// <summary> + /// Gets the start and end dates for the current month. + /// </summary> + /// <param name="now">The current date and time. Defaults to DateTime.Now.</param> + /// <returns>A tuple containing the first and last day of the month.</returns> - private static (DateOnly start, DateOnly end) GetMonthRange() + private static (DateOnly start, DateOnly end) GetMonthRange(DateTime? now = null) { - var now = DateTime.Now; + now ??= DateTime.Now; - var start = new DateOnly(now.Year, now.Month, 1); - var end = new DateOnly(now.Year, now.Month, DateTime.DaysInMonth(now.Year, now.Month)); + var start = new DateOnly(now.Value.Year, now.Value.Month, 1); + var end = new DateOnly(now.Value.Year, now.Value.Month, + DateTime.DaysInMonth(now.Value.Year, now.Value.Month)); return (start, end); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
Guppi.Console/Guppi.Console.csproj
(1 hunks)Guppi.Console/Properties/launchSettings.json
(1 hunks)Guppi.Console/Skills/CalendarSkill.cs
(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- Guppi.Console/Guppi.Console.csproj
🔇 Additional comments (1)
Guppi.Console/Properties/launchSettings.json (1)
5-5
: LGTM! Command line argument update aligns with new feature.
The change from "weather daily" to "calendar month" correctly reflects the new calendar feature being implemented.
Let's verify that the new command exists in the codebase:
✅ Verification successful
✅ Command line argument change is valid and properly implemented
The verification confirms that:
- The
calendar month
command is properly implemented inCalendarSkill.cs
- The command is registered with both text and markdown output options
- The implementation includes proper month calendar display functionality
- The command is correctly integrated into the calendar command hierarchy
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the calendar month command implementation exists
# Expected: Find the command implementation in CalendarSkill.cs
# Search for the calendar month command implementation
ast-grep --pattern 'public class CalendarSkill {
$$$
public $$ Month($$$) {
$$$
}
$$$
}'
# Also check for any command registration
rg -l "calendar.*month" --type cs
Length of output: 138
Script:
#!/bin/bash
# Let's try a broader search to find calendar-related implementations
# Search for any calendar-related classes
ast-grep --pattern 'class $_ {
$$$
calendar
$$$
}'
# Search for calendar command registrations with more flexible pattern
rg -i "command.*calendar" --type cs -A 5
# Search for month-related methods in any class
ast-grep --pattern 'class $_ {
$$$
Month($$$) {
$$$
}
$$$
}'
# Look for calendar-related files
fd -e cs -e fs calendar -i
Length of output: 2281
Script:
#!/bin/bash
# Let's check the specific Month method implementation in CalendarSkill.cs
rg -A 10 "void Month\(" Guppi.Console/Skills/CalendarSkill.cs
rg -A 10 "MonthMarkdown\(" Guppi.Console/Skills/CalendarSkill.cs
# Also verify the command registration structure
rg -A 15 "new Command\(\"calendar\"" Guppi.Console/Skills/CalendarSkill.cs
Length of output: 1874
private static void Month() | ||
{ | ||
(DateOnly start, DateOnly end) = GetMonthRange(); | ||
|
||
AnsiConsoleHelper.TitleRule($":calendar: {start:MMMM yyyy}"); | ||
|
||
var table = new Table(); | ||
table.Border(TableBorder.Rounded); | ||
|
||
table.AddColumn(new TableColumn(new Markup("[yellow]Sun[/]")).RightAligned()); | ||
table.AddColumn(new TableColumn(new Markup("[yellow]Mon[/]")).RightAligned()); | ||
table.AddColumn(new TableColumn(new Markup("[yellow]Tue[/]")).RightAligned()); | ||
table.AddColumn(new TableColumn(new Markup("[yellow]Wed[/]")).RightAligned()); | ||
table.AddColumn(new TableColumn(new Markup("[yellow]Thu[/]")).RightAligned()); | ||
table.AddColumn(new TableColumn(new Markup("[yellow]Fri[/]")).RightAligned()); | ||
table.AddColumn(new TableColumn(new Markup("[yellow]Sat[/]")).RightAligned()); | ||
|
||
// Add empty cells for the last days of the previous month | ||
var row = Enumerable.Range(0, 7).Select(_ => "").ToArray(); | ||
for (var day = start; day <= end; day = day.AddDays(1)) | ||
{ | ||
row[(int)day.DayOfWeek] = day.Day.ToString(); | ||
if (day.DayOfWeek == DayOfWeek.Saturday) | ||
{ | ||
table.AddRow(row); | ||
row = Enumerable.Range(0, 7).Select(_ => "").ToArray(); | ||
} | ||
} | ||
if (end.DayOfWeek != DayOfWeek.Saturday) | ||
table.AddRow(row); | ||
|
||
AnsiConsole.Write(table); | ||
|
||
AnsiConsole.WriteLine(); | ||
AnsiConsoleHelper.Rule("white"); | ||
} | ||
|
||
private static async Task MonthMarkdown() | ||
{ | ||
(DateOnly start, DateOnly end) = GetMonthRange(); | ||
StringBuilder cal = new(); | ||
cal.AppendLine("| Day | Date | Habits | Notes |"); | ||
cal.AppendLine("| --- | ---- | ------ | ----- |"); | ||
for (var day = start; day <= end; day = day.AddDays(1)) | ||
{ | ||
cal.AppendLine($"| **{day:ddd}** | [[{day:yyyy-MM-dd}]] | | |"); | ||
} | ||
|
||
AnsiConsoleHelper.TitleRule($":calendar: {start:MMMM yyyy}"); | ||
|
||
AnsiConsole.WriteLine(); | ||
AnsiConsole.WriteLine(cal.ToString()); | ||
await TextCopy.ClipboardService.SetTextAsync(cal.ToString()); | ||
AnsiConsole.WriteLine(); | ||
AnsiConsole.MarkupLine("[green]:green_circle: Copied to clipboard[/]"); | ||
|
||
AnsiConsole.WriteLine(); | ||
AnsiConsoleHelper.Rule("white"); | ||
} | ||
|
||
private static (DateOnly start, DateOnly end) GetMonthRange() | ||
{ | ||
var now = DateTime.Now; | ||
var start = new DateOnly(now.Year, now.Month, 1); | ||
var end = new DateOnly(now.Year, now.Month, DateTime.DaysInMonth(now.Year, now.Month)); | ||
return (start, end); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add unit tests and improve error handling.
The new calendar functionality needs test coverage and better error handling:
-
Unit tests should cover:
- Calendar display logic
- Markdown generation
- Date range calculations
- Edge cases (month transitions, leap years)
-
Error handling:
- Clipboard operations should be wrapped in try-catch
- User feedback for clipboard failures
Would you like me to help create:
- Unit test templates for the new functionality?
- Error handling implementation for clipboard operations?
private static void Month() | ||
{ | ||
(DateOnly start, DateOnly end) = GetMonthRange(); | ||
|
||
AnsiConsoleHelper.TitleRule($":calendar: {start:MMMM yyyy}"); | ||
|
||
var table = new Table(); | ||
table.Border(TableBorder.Rounded); | ||
|
||
table.AddColumn(new TableColumn(new Markup("[yellow]Sun[/]")).RightAligned()); | ||
table.AddColumn(new TableColumn(new Markup("[yellow]Mon[/]")).RightAligned()); | ||
table.AddColumn(new TableColumn(new Markup("[yellow]Tue[/]")).RightAligned()); | ||
table.AddColumn(new TableColumn(new Markup("[yellow]Wed[/]")).RightAligned()); | ||
table.AddColumn(new TableColumn(new Markup("[yellow]Thu[/]")).RightAligned()); | ||
table.AddColumn(new TableColumn(new Markup("[yellow]Fri[/]")).RightAligned()); | ||
table.AddColumn(new TableColumn(new Markup("[yellow]Sat[/]")).RightAligned()); | ||
|
||
// Add empty cells for the last days of the previous month | ||
var row = Enumerable.Range(0, 7).Select(_ => "").ToArray(); | ||
for (var day = start; day <= end; day = day.AddDays(1)) | ||
{ | ||
row[(int)day.DayOfWeek] = day.Day.ToString(); | ||
if (day.DayOfWeek == DayOfWeek.Saturday) | ||
{ | ||
table.AddRow(row); | ||
row = Enumerable.Range(0, 7).Select(_ => "").ToArray(); | ||
} | ||
} | ||
if (end.DayOfWeek != DayOfWeek.Saturday) | ||
table.AddRow(row); | ||
|
||
AnsiConsole.Write(table); | ||
|
||
AnsiConsole.WriteLine(); | ||
AnsiConsoleHelper.Rule("white"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance calendar display with current day highlight and events.
The calendar display implementation is functional but could be improved:
- Highlight the current day for better user experience
- Include all-day events as mentioned in the PR objectives
- Consider adding padding for better number alignment
Here's a suggested improvement:
private static void Month()
{
(DateOnly start, DateOnly end) = GetMonthRange();
+ var today = DateOnly.FromDateTime(DateTime.Now);
AnsiConsoleHelper.TitleRule($":calendar: {start:MMMM yyyy}");
var table = new Table();
table.Border(TableBorder.Rounded);
+ table.Padding(1, 1);
// ... column definitions ...
var row = Enumerable.Range(0, 7).Select(_ => "").ToArray();
for (var day = start; day <= end; day = day.AddDays(1))
{
- row[(int)day.DayOfWeek] = day.Day.ToString();
+ var dayStr = day.Day.ToString().PadLeft(2);
+ row[(int)day.DayOfWeek] = day == today
+ ? $"[green]{dayStr}[/]"
+ : dayStr;
if (day.DayOfWeek == DayOfWeek.Saturday)
{
table.AddRow(row);
row = Enumerable.Range(0, 7).Select(_ => "").ToArray();
}
}
// ... rest of the method
}
Would you like me to provide an implementation for including all-day events in the calendar display?
Committable suggestion skipped: line range outside the PR's diff.
Month calendars
Fixes #206
Summary by CodeRabbit