Jalaali (Persian) Date input support #1205

inamvar · 2021-04-12T11:55:04Z

resolving issue #1204

Terminal.Gui/Views/DateField.cs

tznind · 2021-04-27T07:45:47Z

Is this something that could be done with inheritance? By making GetShortFormat and maybe the Date property virtual (perhaps combined with making shortFormat and longFormat public (or readonly/protected)?

Then users could create their own custom date formats and calendars types? Could even have a Jalaali version of DateField (either in core or Scenarios) to show how to implement.

inamvar · 2021-04-27T08:05:57Z

Is this something that could be done with inheritance? By making GetShortFormat and maybe the Date property virtual (perhaps combined with making shortFormat and longFormat public (or readonly/protected)?

Then users could create their own custom date formats and calendars types? Could even have a Jalaali version of DateField (either in core or Scenarios) to show how to implement.

Sounds good! It needs to change some methods identifiers in DateField class and set them as virtual methods to be able to override them in the inherited class.

tig · 2021-04-27T15:45:20Z

Is this something that could be done with inheritance? By making GetShortFormat and maybe the Date property virtual (perhaps combined with making shortFormat and longFormat public (or readonly/protected)?
Then users could create their own custom date formats and calendars types? Could even have a Jalaali version of DateField (either in core or Scenarios) to show how to implement.

Sounds good! It needs to change some methods identifiers in DateField class and set them as virtual methods to be able to override them in the inherited class.

this seems like a good approach.

tig

@inamvar,

We currently have zero unit tests for DateField. Via other tests, it has about 53% code coverage.

I'd like to ask you to add a DateFieldTest.cs module with some tests that would get this closer to 80%. Would you please do this as part of this PR?

Examples:

the constructors
functions like GetLongFormat, 'GetDate', etc...
anywhere you see any sort of logic that maniuplates stuff

Thanks!!!!

tig · 2021-04-27T17:15:00Z

Terminal.Gui/Views/DateField.cs

 		/// <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>
+		public DateField (int x, int y, DateTime date, bool isShort = false, bool isJalaali = false) : base (x, y, isShort ? 10 : 12, "")


A few unit tests that prove the various constructors work as they should would be good.

tig · 2021-04-27T17:17:36Z

Terminal.Gui/Views/DateField.cs

 			DateChanged?.Invoke (args);
 		}
+
+		string ToJalaaliString (string format, DateTime date)


You should have a unit test for this.

tig · 2021-05-30T13:33:45Z

Hi @inamvar - We'd love to integrate this PR. Will you have a chance soon to address the changes requested?

tig · 2021-07-21T16:03:46Z

@inamvar - are you still interested in helping with this?

tig · 2022-10-20T14:29:17Z

@inamvar - I'm marking this as a Draft PR. Hope you come back some time and help us get this merged in!

Copilot

Pull Request Overview

This PR adds support for the Jalaali (Persian) calendar to the DateField component, allowing users to input and display dates using the Persian calendar system. The implementation includes new constructor parameters, date conversion logic, and formatting capabilities.

Key Changes

Added isJalaali parameter to DateField constructors to enable Persian calendar mode
Implemented ToJalaaliString method to convert Gregorian dates to Jalaali format
Modified SetText method to handle Jalaali date parsing and validation

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 22 comments.

File	Description
Terminal.Gui/Views/DateField.cs	Core implementation of Jalaali calendar support including new constructors, date conversion logic, and formatting method
UICatalog/Scenarios/Text.cs	Minor whitespace-only change with no functional impact

Comments suppressed due to low confidence (2)

Terminal.Gui/Views/DateField.cs:289

Logic error: The variable d is undefined in the Jalaali code path (lines 266-285). When isJalaali is true, the code skips the string d = GetDate(...) declaration on line 280, but line 287 still attempts to use d in DateTime.TryParseExact(d, format, ...), which will cause a compilation error.

			if (isJalaali) {
				try {
					PersianCalendar pc = new PersianCalendar ();
					if (year < 10) {
						year += 1400;
					}
					result = pc.ToDateTime (year, month, day, 12, 0, 0, 0);
					year = result.Year;
					month = result.Month;
					day = result.Day;
				} catch {
					return false;
				}
			} else {
				string d = GetDate (month, day, year, frm);

				if (!DateTime.TryParseExact (d, Format, CultureInfo.CurrentCulture, DateTimeStyles.None, out result) ||
					!isValidDate)
					return false;
			}

			if (!DateTime.TryParseExact (d, format, CultureInfo.CurrentCulture, DateTimeStyles.None, out DateTime result) ||
				!isValidDate)
				return false;

Terminal.Gui/Views/DateField.cs:289

Unreachable code: Lines 287-289 contain duplicate validation logic that can never be reached when isJalaali is true (because line 278 already returns false on error). When isJalaali is false, these lines are also unreachable because line 284 already returns false if validation fails. This entire block appears to be dead code left over from the original implementation and should be removed.

			if (!DateTime.TryParseExact (d, format, CultureInfo.CurrentCulture, DateTimeStyles.None, out DateTime result) ||
				!isValidDate)
				return false;

Copilot · 2025-11-12T19:01:18Z

Terminal.Gui/Views/DateField.cs

+					this.Text = ToJalaaliString (Format, date);
+				} else {
+					this.Text = value.ToString (Format);
+				}
+				var args = new DateTimeEventArgs<DateTime> (oldData, value, Format);


Case sensitivity issue: Format should be format (lowercase). The property format is defined as a lowercase property expression, not Format.

Suggested change

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 · 2025-11-12T19:01:19Z

Terminal.Gui/Views/DateField.cs

 			this.isShort = isShort;
+
 			Date = date;
 			CursorPosition = 1;


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.

Suggested change

CursorPosition = 1;

Copilot · 2025-11-12T19:01:19Z

Terminal.Gui/Views/DateField.cs

 		/// </summary>
 		/// <param name="date"></param>
-		public DateField (DateTime date) : base ("")
+		/// <param name="isJalaali"></param>


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.

Copilot · 2025-11-12T19:01:19Z

Terminal.Gui/Views/DateField.cs

+
+			this.isJalaali = isJalaali;
+			if (!isJalaali)
+				this.isShort = true;
+			Width = FieldLen + 2;
+


[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 · 2025-11-12T19:01:19Z

Terminal.Gui/Views/DateField.cs

+
+
+			var year = pc.GetYear (date);
+			var month = pc.GetMonth (date);
+			var day = pc.GetDayOfMonth (date);
+
+


[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.

Suggested change

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 · 2025-11-12T19:01:23Z

Terminal.Gui/Views/DateField.cs

+					}
+					result = pc.ToDateTime (year, month, day, 12, 0, 0, 0);
+					year = result.Year;
+					month = result.Month;


This assignment to month is useless, since its value is never read.

Suggested change

month = result.Month;

Copilot · 2025-11-12T19:01:23Z

Terminal.Gui/Views/DateField.cs

+					result = pc.ToDateTime (year, month, day, 12, 0, 0, 0);
+					year = result.Year;
+					month = result.Month;
+					day = result.Day;


This assignment to day is useless, since its value is never read.

Suggested change

day = result.Day;

Copilot · 2025-11-12T19:01:24Z

Terminal.Gui/Views/DateField.cs

 		string sepChar;
 		string longFormat;
 		string shortFormat;
+		bool isJalaali;


Field 'isJalaali' can be 'readonly'.

Suggested change

bool isJalaali;

readonly bool isJalaali;

Copilot · 2025-11-12T19:01:24Z

Terminal.Gui/Views/DateField.cs

+				} catch {
+					return false;


Generic catch clause.

Suggested change

} catch {

return false;

} catch (ArgumentOutOfRangeException) {

return false;

} catch (FormatException) {

return false;

Copilot · 2025-11-12T19:01:24Z

Terminal.Gui/Views/DateField.cs

+				if (isJalaali) {
+
+					this.Text = ToJalaaliString (Format, date);
+				} else {
+					this.Text = value.ToString (Format);
+				}


Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.

Suggested change

if (isJalaali) {

this.Text = ToJalaaliString (Format, date);

} else {

this.Text = value.ToString (Format);

}

this.Text = isJalaali ? ToJalaaliString (Format, date) : value.ToString (Format);

inamvar added 2 commits April 12, 2021 16:22

feat: Jalaali (Persian) Date input support

a07a5e1

fix: clean scenario Text input sample

12f6368

tig mentioned this pull request Apr 13, 2021

Jalaali(Persian) Date input support #1204

Open

tig requested changes Apr 20, 2021

View reviewed changes

Terminal.Gui/Views/DateField.cs Show resolved Hide resolved

tig requested changes Apr 27, 2021

View reviewed changes

tig marked this pull request as draft October 20, 2022 14:29

Merge branch 'develop' into JalaaliDateInput

db53ad5

inamvar marked this pull request as ready for review October 23, 2022 13:08

inamvar requested a review from migueldeicaza as a code owner October 23, 2022 13:08

Merge branch 'develop' into JalaaliDateInput

bf1915c

tznind mentioned this pull request Nov 13, 2022

layout mirroring Right to Left flow #2221

Closed

Merge branch 'v1_develop' into JalaaliDateInput

3c28f6a

tig requested a review from Copilot November 12, 2025 18:53

tig assigned Copilot Nov 12, 2025

tig removed the request for review from migueldeicaza November 12, 2025 18:54

Copilot started reviewing on behalf of tig November 12, 2025 18:54 View session

Copilot finished reviewing on behalf of tig November 12, 2025 18:57

Copilot AI reviewed Nov 12, 2025

View reviewed changes

Jalaali (Persian) Date input support #1205

Are you sure you want to change the base?

Jalaali (Persian) Date input support #1205

Conversation

inamvar commented Apr 12, 2021

Uh oh!

Uh oh!

tznind commented Apr 27, 2021

Uh oh!

inamvar commented Apr 27, 2021

Uh oh!

tig commented Apr 27, 2021

Uh oh!

tig left a comment

Choose a reason for hiding this comment

Uh oh!

tig Apr 27, 2021

Choose a reason for hiding this comment

Uh oh!

tig Apr 27, 2021

Choose a reason for hiding this comment

Uh oh!

tig commented May 30, 2021

Uh oh!

tig commented Jul 21, 2021

Uh oh!

tig commented Oct 20, 2022

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes

Reviewed Changes

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants