Improving experience with format and parse.
#9045
Conversation
jdunkerley
force-pushed
the
wip/jd/format-parse
branch
from
c578152
to
1e03dec
Compare
jdunkerley
requested review from
radeusgd,
GregoryTravis and
AdRiley
as code owners
jdunkerley
added
the
CI: No changelog needed
| @@ -124,7 +124,7 @@ type Date_Time_Formatter | |||
| Date.parse "1 Nov '95" "d MMM ''yy{2099}" == (Date.new 2095 11 01) | |||
| @locale Locale.default_widget | |||
| from_simple_pattern : Text -> Locale -> Date_Time_Formatter ! Date_Time_Format_Parse_Error | |||
| from_simple_pattern pattern:Text locale:Locale=Locale.default = | |||
| from_simple_pattern pattern:Text='' locale:Locale=Locale.default = | |||
There was a problem hiding this comment.
minor thing: I see above in Numbers.format we have "" and here '', shall we try to 'standardize' which one we prefer for empty strings? Or does this depend on the context?
How does the text input widget work? Does it always use '? Does it support escapes?
There was a problem hiding this comment.
I want to move to:
- Use single quotes by default.
- Use double quotes if it would avoid escaping.
- Use escaped single quotes.
No the text input doesn't do that it just uses escaped single quotes.
| format = Date_Time_Formatter.from_simple_pattern 'd MMMM yyyy' locale=Locale.france | ||
| label = 'd MMMM yyyy - with custom Locale (e.g. ' + date.format format + ')' | ||
| code = "(Date_Time_Formatter.from_simple_pattern 'd MMMM yyyy' locale=Locale.france)" | ||
| Option label code [["pattern", make_single_choice formats], ["locale", Locale.default_widget]] |
There was a problem hiding this comment.
Shouldn't these nested widgets be set automatically?
Or if we set the 'toplevel' widget, than all inner ones must also be set manually to work?
Asking mostly to understand and use the correct practice myself when adding any widgets.
There was a problem hiding this comment.
Yes the top level ones will be set automatically (when a bug is fixed).
However in this case I am deliberately adding the inner ones so I can be consistent with outer one and be value type determining what is added.
There was a problem hiding this comment.
Ahh, I see! I did not guess that.
I think a comment like 'Overriding the widget for patterns to ensure that the provided formats are the same as on the top level' could clarify this then
distribution/lib/Standard/Base/0.0.0-dev/src/Widget_Helpers.enso
Outdated
Show resolved
Hide resolved
| formatter = make_value_formatter_for_value_type self.value_type locale format | ||
| Column_Ops.map_over_storage self formatter make_string_builder on_problems=Problem_Behavior.Report_Error | ||
| new_column | ||
| @format (make_format_chooser include_number=(self.value_type.is_numeric || self.value_type==Value_Type.Mixed) include_date=(self.value_type==Value_Type.Date || self.value_type==Value_Type.Mixed) include_date_time=(self.value_type.is_date_time || self.value_type==Value_Type.Mixed) include_time=(self.value_type==Value_Type.Time_Of_Day || self.value_type==Value_Type.Mixed) include_boolean=(self.value_type.is_boolean || self.value_type==Value_Type.Mixed)) |
There was a problem hiding this comment.
I'm tempted to refactor this to take one argument type Typed_Format_Chooser_Settings and have a separate function that computes it based on the self.value_type.
It looks a little bit 'unhygienic' to have such a huge expression in the @ annotation.
Alternatively, we could skip the Typed_Format_Chooser_Settings type, but at least create a helper function make_format_chooser_for_typed_column self and move this logic into there.
(Probably a spurious optimization but I also don't feel as good about so many
self.value_typereferences - while not exactly very expensive, they do more than just accessing a field)
There was a problem hiding this comment.
I think a method in table Widget_Helpers which took a value type and returned it makes sense.
Pull Request Description
Add format dropdown to
Number.format.Support case insensitive month names and abbreviations in dates.
https://github.com/enso-org/enso/assets/4699705/4dbd8755-e1c2-4207-a8a1-65b427ca4fab
Improve locale dropdown for
parse_dateandparse_date_time.Added dropdown to
Table.parseand amended so now doesn't acceptNothing(using empty string instead).Added dropdown to
Table.formatand amended so now doesn't acceptNothing(using empty string instead).Altered
Column.parseto not acceptNothingand added drop down for format.Altered
Column.formatto not acceptNothingand added drop down for format conditional on type.Improved the locale date/time format drop to have the suggested formats too.
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
./run ide build.