-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Add new langNumFmtFunc in numfmt
#1895
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: wushiling50 <2531010934@qq.com>
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.
Thanks for your PR. Could you add unit test for this changes?
No problem, it was my oversight, I'll add unit tests for these changes |
Signed-off-by: wushiling50 <2531010934@qq.com>
Signed-off-by: wushiling50 <2531010934@qq.com>
Signed-off-by: wushiling50 <2531010934@qq.com>
@@ -3613,3 +3613,74 @@ func TestNumFmt(t *testing.T) { | |||
assert.Equal(t, ErrUnsupportedNumberFormat, err) | |||
assert.False(t, changeNumFmtCode) | |||
} | |||
|
|||
func TestLangNumFmtFunc(t *testing.T) { |
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.
Did you test each built-in number format with different data time pattern settings on real Office application? The cells in these workbooks are formatted with built-in and built-in language related number format.
We can tested by open it in spreadsheet application with different language and data time pattern settings, and get some test cases just like this:
Experimental evaluation: date time number format
Built-in number format code & built-in language number format code ID range 27 ~ 36, 50 ~ 81:
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.
So, what i should do is:
- Place the numfmt.xlsx file inside the ./test directory.
- In the tests, set the langNumFmt for the cells in sequence, with each column corresponding to a different region.
- Retrieve the cell values and compare them to ensure they are correct.
It's not just about the new additions i have made, but also about including zh-cn and en-us as well,right?
In addition to langNumFmt, I also need to do the part of Built-in number format code, right?
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 needn't add new workbook to the test directory, the workbook just for test, open the workbook in the real spreadsheet apps to observe formats result in different data time pattern settings, and using the formatted result as expected result in the unit test.
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.
Understood, thank you.
func main() {
f := excelize.NewFile()
numfmt1 := "e\"年\"m\"月\"d\"日\""
style1, err := f.NewStyle(&excelize.Style{
CustomNumFmt: &numfmt1,
})
if err != nil {
fmt.Println(err)
return
}
f.SetCellStyle("Sheet1", "A2", "A2", style1)
f.SetCellValue("Sheet1", "A2", 45050)
cellValue1, _ := f.GetCellValue("Sheet1", "A2")
fmt.Printf("1:%v\n", cellValue1)
numfmt2 := "yyyy\"年\"m\"月\"d\"日\""
style2, err := f.NewStyle(&excelize.Style{
CustomNumFmt: &numfmt2,
})
if err != nil {
fmt.Println(err)
return
}
f.SetCellStyle("Sheet1", "B2", "B2", style2)
f.SetCellValue("Sheet1", "B2", 45050)
cellValue2, _ := f.GetCellValue("Sheet1", "B2")
fmt.Printf("2:%v\n", cellValue2)
numfmt3 := "[$-411]ggge\"年\"m\"月\"d\"日\""
style3, err := f.NewStyle(&excelize.Style{
CustomNumFmt: &numfmt3,
})
if err != nil {
fmt.Println(err)
return
}
f.SetCellStyle("Sheet1", "C2", "C2", style3)
f.SetCellValue("Sheet1", "C2", 45050)
cellValue3, _ := f.GetCellValue("Sheet1", "C2")
fmt.Printf("3:%v\n", cellValue3)
if err := f.SaveAs("demo/Book1.xlsx"); err != nil {
fmt.Println(err)
}
}
func main() {
f := excelize.NewFile()
numfmt4 := "ว-ดดด-ปป"
style4, err := f.NewStyle(&excelize.Style{
CustomNumFmt: &numfmt4,
})
if err != nil {
fmt.Println(err)
return
}
f.SetCellStyle("Sheet1", "D2", "D2", style4)
f.SetCellValue("Sheet1", "D2", 45050)
cellValue4, _ := f.GetCellValue("Sheet1", "D2")
fmt.Printf("4:%v\n", cellValue4)
if err := f.SaveAs("demo/Book1.xlsx"); err != nil {
fmt.Println(err)
} These problems are leading to the failure of the "zh-tw" and "th-th" related tests, and if you are agreeable, I can provide the code for the "ja-jp" and "ko-kr" parts first, and within the getBuiltInNumFmtCode function, I can set a TODO comment. Both "ja-jp" part and ko-kr" part have passed the tests successfully. |
Good job. I think we need to resolve this number format code parse or evaluate the issue before merging this. Maybe it will be related to the NFP library, I'm not sure. |
79958aa
to
0c3dfb1
Compare
PR Details
Support for more built-in langNumFmt allows
GetCellValue
to fetch dates and times in more localizationsDescription
Related Issue
#1885
Motivation and Context
When I using the following code, I found that the current
CultureInfo
type only supportsCultureNameZhCN
andCultureNameEnUS
, which makes it impossible for me to obtain the time and date formats of other regions through theGetCellValue
method.How Has This Been Tested
Add Unit Test
Types of changes
Checklist