feat: add support for Go-style date parsing and formatting in nepalitime package#30
feat: add support for Go-style date parsing and formatting in nepalitime package#30codernirdesh wants to merge 1 commit intoopensource-nepal:mainfrom
Conversation
|
Thank you @codernirdesh for your contribution. We will review the code shortly. |
|
@codernirdesh, Your tests are failing on CI, could you please check on it. |
|
@codernirdesh Thanks for the contribution and the fantastic write-up! Quick note: we just bumped the Go version to 1.24 (used to be 1.19). Could you grab the latest from main and tweak your changes if required? |
sugat009
left a comment
sugat009
left a comment
There was a problem hiding this comment.
Thanks for the contribution, @codernirdesh . I've found a few issues.
@aj3sh and I originally considered implementing all this functionality directly in the Parse and Format functions. However, splitting into two separate functions aligns better with the single responsibility principle.
To make our code more similar to the time library in the future, we should eventually move the changes in this PR into the Parse and Format functions, while renaming the current functions to ParseLayout and FormatLayout. Since this would be a breaking change requiring a major version release, the interim solution is to implement these changes as ParseLayout and FormatLayout functions now, then rename them in a future major release.
| ### New Feature: Go-Style Layout Parsing | ||
|
|
||
| The `nepalitime` package now supports parsing Go-style layouts for dates and times. This feature allows you to parse the current system date using a layout string that follows Go's native date and time formatting conventions. |
There was a problem hiding this comment.
issue: I feel like this section is not necessary. Right now, the feature is new and makes sense, but down the line, after some time, it won't be a "new feature" anymore and this section will need editing.
| // Updated ParseGoLayout to only use layout | ||
| func ParseGoLayout(layout string) (*NepaliTime, error) { | ||
| currentTime := time.Now() | ||
| parsedTime, err := time.Parse(layout, currentTime.Format(layout)) |
There was a problem hiding this comment.
issue: This function would parse the current time, which is constantly moving, to a given layout. What if I wanted to parse a custom date string like in the example given in the ticket?
| func (obj *NepaliFormatter) FormatGoLayout(layout string) string { | ||
| englishTime := obj.nepaliTime.GetEnglishTime() | ||
| return englishTime.Format(layout) | ||
| } |
There was a problem hiding this comment.
issue: the target of formatting is intended to be for the Nepali dates and not the English date equivalents. For example: when I do nepaliFormatter.FormatGoLayout('2006-02-01'), I'd be expecting the Nepali date to be formatted in the 2006-02-01 format, which would be 2056-10-18.
|
|
||
| layout := "Jan 2, 2006 at 3:04pm (MST)" | ||
|
|
||
| npTime, err := nepalitime.ParseGoLayout(layout) |
There was a problem hiding this comment.
suggestion: Let's rename this to a simpler ParseLayout. I feel like the Go is a given.
CC: @aj3sh
| layout := "Jan 2, 2006 at 3:04pm (MST)" | ||
|
|
||
| formatter := nepalitime.NewFormatter(npTime) | ||
| formatted := formatter.FormatGoLayout(layout) |
There was a problem hiding this comment.
suggestion: similar to above. Let's rename this to FormatLayout.
3 participants
This pull request introduces support for Go-style date and time layout parsing and formatting in the
nepalitimepackage, along with corresponding updates to the documentation and test cases. The key changes include the addition ofParseGoLayoutandFormatGoLayoutmethods, tests for these new features, and updates to theREADME.mdto document their usage.New Feature: Go-Style Layout Parsing and Formatting
ParseGoLayoutfunction innepalitime/parser.goto parse the current system date using Go-style layouts. This provides seamless integration with Go's native date and time formatting conventions.FormatGoLayoutmethod innepalitime/formatter.goto formatNepaliTimeobjects into Go-style layout strings.Documentation Updates
README.mdwith examples for parsing and formatting Go-style layouts, including detailed explanations and code snippets. [1] [2]Test Enhancements
TestParseGoLayoutinnepalitime/parser_test.goto validate the functionality ofParseGoLayout.TestFormatGoLayoutinnepalitime/formatter_test.goto ensure theFormatGoLayoutmethod formats dates correctly.Minor Changes
timepackage tonepalitime/parser.goto support the new parsing functionalityFixes Support GO parsing and formatting layout #6