feat: add Character and Year converters (#1017) - #1087
Conversation
Signed-off-by: BigDataDZ <76271875+BigDataDZ@users.noreply.github.com>
| @Override | ||
| public WriteCellData<?> convertToExcelData( | ||
| Character value, ExcelContentProperty contentProperty, GlobalConfiguration globalConfiguration) { | ||
| return new WriteCellData<>(String.valueOf(value)); |
There was a problem hiding this comment.
Please consider directly using value.toString().
| private static DateTimeFormatter getFormatter( | ||
| ExcelContentProperty contentProperty, GlobalConfiguration globalConfiguration) { | ||
| if (contentProperty == null || contentProperty.getDateTimeFormatProperty() == null) { | ||
| return DEFAULT_FORMATTER; | ||
| } | ||
| return DateTimeFormatter.ofPattern( | ||
| contentProperty.getDateTimeFormatProperty().getFormat(), globalConfiguration.getLocale()); | ||
| } |
There was a problem hiding this comment.
Since the caching logic of DateTimeFormatter has already been implemented in DateUtils through DATE_TIME_FORMATTER_THREAD_LOCAL, can we integrate it into DateUtils (for example, DateUtils#parseYear, #format(Year...))?
| */ | ||
| public class YearStringConverter implements Converter<Year> { | ||
|
|
||
| private static final DateTimeFormatter DEFAULT_FORMATTER = DateTimeFormatter.ofPattern("uuuu"); |
There was a problem hiding this comment.
While "uuuu" can output a minus sign when formatting, it enforces a fixed 4-character width during parsing and will throw an exception on signed years like "-45". As a result, its parsing behavior is practically no different from "yyyy".
If the intent is to support BCE/negative years and variable-length inputs, changing the pattern to "u" would be better. Otherwise, if negative years are not in scope, would it be cleaner to stick with the conventional "yyyy"?
Signed-off-by: BigDataDZ <76271875+BigDataDZ@users.noreply.github.com>
| public static final String DATE_FORMAT_19_FORWARD_SLASH = "yyyy/MM/dd HH:mm:ss"; | ||
| public static final String TIME_FORMAT_5 = "HH:mm"; | ||
| public static final String TIME_FORMAT_8 = "HH:mm:ss"; | ||
| public static final String DEFAULT_YEAR_FORMAT = "yyyy"; |
There was a problem hiding this comment.
Recommended to follow the field naming convention.
Signed-off-by: BigDataDZ <76271875+BigDataDZ@users.noreply.github.com>
|
Done in 254a11e — renamed to |
2 participants
Purpose of the pull request
Implements two commonly used JDK 8 compatible converters requested by the community task #1017:
java.lang.Characterandjava.time.Year. Proposal comments:#1017 (comment) and the amendment below it
(
UUID/YearMonth/Instantwere proposed by other volunteers in parallel and are left tothem).
Related: #1017
What's changed?
CharacterStringConverter(org.apache.fesod.sheet.converters.charconverter): bidirectionalCharacter<-> STRING. Reading an empty string yieldsnull; strings longer than one characterthrow (wrapped into
ExcelDataConvertExceptionby the framework) instead of being silentlytruncated.
YearStringConverter(org.apache.fesod.sheet.converters.year): bidirectionalYear<->STRING, default format
uuuu, honors@DateTimeFormatcustom patterns andGlobalConfiguration.locale.Both follow the existing
*StringConverterpattern (e.g.LocalTimeStringConverter) and areregistered in
DefaultConverterLoaderunderputAllConverter,putWriteConverterandputWriteStringConverter, so they work on read, xlsx write and CSV write alike.Each converter ships with JUnit 5 unit tests (11 tests total: type keys, round-trips, default and
custom formats, and error cases).
Checklist