-
Notifications
You must be signed in to change notification settings - Fork 731
Jalaali (Persian) Date input support #1205
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
base: v1_develop
Are you sure you want to change the base?
Changes from all commits
a07a5e1
12f6368
db53ad5
bf1915c
3c28f6a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,10 +5,10 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Licensed under the MIT license | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using NStack; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using System; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using System.Globalization; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using System.Linq; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using NStack; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| namespace Terminal.Gui { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -25,6 +25,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| string sepChar; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| string longFormat; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| string shortFormat; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bool isJalaali; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| int fieldLen => isShort ? shortFieldLen : longFieldLen; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| string format => isShort ? shortFormat : longFormat; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -47,9 +48,14 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// <param name="y">The y coordinate.</param> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// <param name="date">Initial date contents.</param> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// <param name="isShort">If true, shows only two digits for the year.</param> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public DateField (int x, int y, DateTime date, bool isShort = false) : base (x, y, isShort ? 10 : 12, "") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// <param name="isJalaali">If true, parse will convert jalaali input fo georgian date</param> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// <param name="isJalaali">If true, parse will convert jalaali input fo georgian date</param> | |
| /// <param name="isJalaali">If true, parse will convert jalaali input to Gregorian date</param> |
tig marked this conversation as resolved.
Show resolved
Hide resolved
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.
A few unit tests that prove the various constructors work as they should would be good.
Copilot
AI
Nov 12, 2025
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.
[nitpick] Unnecessary blank lines: Excessive vertical whitespace on lines 54, 58, 73, 78 reduces code readability. According to the Mono Coding Guidelines (referenced in CONTRIBUTING.md), blank lines should be used sparingly and purposefully to separate logical sections of code.
| { | |
| this.isShort = isShort; | |
| this.isJalaali = isJalaali; | |
| Initialize (date, isShort); | |
| } | |
| public class DateField : TextField { | |
| this.isJalaali = isJalaali; | |
| Initialize (date, isShort); |
Copilot
AI
Nov 12, 2025
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.
Missing XML documentation: The new isJalaali parameter lacks proper XML documentation. According to the CONTRIBUTING.md guidelines, all public API parameters must have clear, concise documentation using <param> tags.
Check failure on line 77 in Terminal.Gui/Views/DateField.cs
GitHub Actions / Non-Parallel Unit Tests (ubuntu-latest)
The name 'FieldLen' does not exist in the current context
Copilot
AI
Nov 12, 2025
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.
Case sensitivity issue: FieldLen should be fieldLen (lowercase). The property fieldLen is defined as a lowercase property expression, not FieldLen.
| Width = FieldLen + 2; | |
| Width = fieldLen + 2; |
Copilot
AI
Nov 12, 2025
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.
[nitpick] Unnecessary blank lines: Excessive vertical whitespace reduces code readability. According to the Mono Coding Guidelines (referenced in CONTRIBUTING.md), blank lines should be used sparingly and purposefully to separate logical sections of code.
Copilot
AI
Nov 12, 2025
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.
Duplicate CursorPosition assignment. Line 88 sets CursorPosition = 1; but it's set again on line 93. This is redundant - one of these assignments should be removed.
| CursorPosition = 1; |
Check failure on line 178 in Terminal.Gui/Views/DateField.cs
GitHub Actions / Non-Parallel Unit Tests (ubuntu-latest)
The name 'Format' does not exist in the current context
Check failure on line 180 in Terminal.Gui/Views/DateField.cs
GitHub Actions / Non-Parallel Unit Tests (ubuntu-latest)
The name 'Format' does not exist in the current context
Copilot
AI
Nov 12, 2025
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.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
| if (isJalaali) { | |
| this.Text = ToJalaaliString (Format, date); | |
| } else { | |
| this.Text = value.ToString (Format); | |
| } | |
| this.Text = isJalaali ? ToJalaaliString (Format, date) : value.ToString (Format); |
Copilot
AI
Nov 12, 2025
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.
Case sensitivity issue: Format should be format (lowercase). The property format is defined as a lowercase property expression, not Format.
| this.Text = ToJalaaliString (Format, date); | |
| } else { | |
| this.Text = value.ToString (Format); | |
| } | |
| var args = new DateTimeEventArgs<DateTime> (oldData, value, Format); | |
| this.Text = ToJalaaliString (format, date); | |
| } else { | |
| this.Text = value.ToString (format); | |
| } | |
| var args = new DateTimeEventArgs<DateTime> (oldData, value, format); |
Copilot
AI
Nov 12, 2025
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.
Case sensitivity issue: Format should be format (lowercase). The property format is defined as a lowercase property expression, not Format.
| this.Text = ToJalaaliString (Format, date); | |
| } else { | |
| this.Text = value.ToString (Format); | |
| } | |
| var args = new DateTimeEventArgs<DateTime> (oldData, value, Format); | |
| this.Text = ToJalaaliString (format, date); | |
| } else { | |
| this.Text = value.ToString (format); | |
| } | |
| var args = new DateTimeEventArgs<DateTime> (oldData, value, format); |
Copilot
AI
Nov 12, 2025
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.
Case sensitivity issue: Format should be format (lowercase). The property format is defined as a lowercase property expression, not Format.
| this.Text = ToJalaaliString (Format, date); | |
| } else { | |
| this.Text = value.ToString (Format); | |
| } | |
| var args = new DateTimeEventArgs<DateTime> (oldData, value, Format); | |
| this.Text = ToJalaaliString (format, date); | |
| } else { | |
| this.Text = value.ToString (format); | |
| } | |
| var args = new DateTimeEventArgs<DateTime> (oldData, value, format); |
Copilot
AI
Nov 12, 2025
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.
[nitpick] Unnecessary blank lines: Excessive vertical whitespace reduces code readability. According to the Mono Coding Guidelines (referenced in CONTRIBUTING.md), blank lines should be used sparingly and purposefully to separate logical sections of code.
Copilot
AI
Nov 12, 2025
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.
Magic number without explanation: The value 1400 on line 270 appears to be a base year for Jalaali calendar conversion when the year is less than 10, but there's no comment explaining this logic. This makes the code harder to understand and maintain. Consider adding a comment explaining why 1400 is used (likely the current Jalaali century).
| if (year < 10) { | |
| if (year < 10) { | |
| // If the year is less than 10, assume it is a two-digit year in the current Jalaali century (1400s). | |
| // The Jalaali century 1400 started in 2021 Gregorian. |
Copilot
AI
Nov 12, 2025
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.
This assignment to month is useless, since its value is never read.
| month = result.Month; |
Copilot
AI
Nov 12, 2025
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.
This assignment to year is useless, since its value is never read.
| year = result.Year; | |
| month = result.Month; | |
| day = result.Day; | |
| // Removed unused assignments to year, month, and day. |
Copilot
AI
Nov 12, 2025
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.
This assignment to day is useless, since its value is never read.
| day = result.Day; |
Copilot
AI
Nov 12, 2025
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.
Generic catch clause.
| } catch { | |
| return false; | |
| } catch (ArgumentOutOfRangeException) { | |
| return false; | |
| } catch (FormatException) { | |
| return false; |
Check failure on line 287 in Terminal.Gui/Views/DateField.cs
GitHub Actions / Non-Parallel Unit Tests (ubuntu-latest)
A local variable or function named 'result' is already defined in this scope
Copilot
AI
Nov 12, 2025
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.
Missing XML documentation: The new ToJalaaliString method lacks XML documentation. According to the CONTRIBUTING.md guidelines, all methods should have clear, concise documentation including a <summary> tag and documentation for parameters and return values.
| /// <summary> | |
| /// Converts the specified <see cref="DateTime"/> to a Jalaali (Persian) date string using the given format. | |
| /// </summary> | |
| /// <param name="format">The format string to use for the Jalaali date. If null or empty, defaults to " yyyy/MM/dd".</param> | |
| /// <param name="date">The <see cref="DateTime"/> value to convert.</param> | |
| /// <returns>A string representing the Jalaali date in the specified format.</returns> |
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.
You should have a unit test for this.
Copilot
AI
Nov 12, 2025
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.
Local scope variable 'format' shadows DateField.format.
Copilot
AI
Nov 12, 2025
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.
Local scope variable 'date' shadows DateField.date.
| string ToJalaaliString (string format, DateTime date) | |
| { | |
| if (string.IsNullOrEmpty (format)) format = " yyyy/MM/dd"; | |
| PersianCalendar pc = new PersianCalendar (); | |
| var year = pc.GetYear (date); | |
| var month = pc.GetMonth (date); | |
| var day = pc.GetDayOfMonth (date); | |
| string ToJalaaliString (string format, DateTime inputDate) | |
| { | |
| if (string.IsNullOrEmpty (format)) format = " yyyy/MM/dd"; | |
| PersianCalendar pc = new PersianCalendar (); | |
| var year = pc.GetYear (inputDate); | |
| var month = pc.GetMonth (inputDate); | |
| var day = pc.GetDayOfMonth (inputDate); |
Copilot
AI
Nov 12, 2025
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.
[nitpick] Unnecessary blank lines: Excessive vertical whitespace reduces code readability. According to the Mono Coding Guidelines (referenced in CONTRIBUTING.md), blank lines should be used sparingly and purposefully to separate logical sections of code.
| var year = pc.GetYear (date); | |
| var month = pc.GetMonth (date); | |
| var day = pc.GetDayOfMonth (date); | |
| var year = pc.GetYear (date); | |
| var month = pc.GetMonth (date); | |
| var day = pc.GetDayOfMonth (date); |
Copilot
AI
Nov 12, 2025
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.
[nitpick] Unnecessary blank lines: Excessive vertical whitespace reduces code readability. According to the Mono Coding Guidelines (referenced in CONTRIBUTING.md), blank lines should be used sparingly and purposefully to separate logical sections of code.
| var year = pc.GetYear (date); | |
| var month = pc.GetMonth (date); | |
| var day = pc.GetDayOfMonth (date); | |
| var year = pc.GetYear (date); | |
| var month = pc.GetMonth (date); | |
| var day = pc.GetDayOfMonth (date); |
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.
Field 'isJalaali' can be 'readonly'.