On Wed, Jan 15, 2020 at 11:20 AM Arthur Zakirov <zaar...@gmail.com> wrote:
>
> I have some couple picky notes.
>
>
Thanks for the review.
> > + if (name_len != norm_len)
> > + pfree(norm_name);
>
> I'm not sure here. Is it save to assume that if it was allocated a new
> norm_name name_len and norm_len always will differ?
>
Good call, that check can be more robust.
>
> > static const char *const months_full[] = {
> > "January", "February", "March", "April", "May", "June", "July",
> > - "August", "September", "October", "November", "December", NULL
> > + "August", "September", "October", "November", "December"
> > };
>
> Unfortunately I didn't see from the patch why it was necessary to remove
> last null element from months_full, days_short, adbc_strings,
> adbc_strings_long and other arrays. I think it is not good because
> unnecessarily increases the patch and adds code like the following:
>
> > + from_char_seq_search(&value, &s, months,
> localized_names,
> > +
> ONE_UPPER, MAX_MON_LEN, n, have_error,
> > +
> lengthof(months_full));
>
> Here it passes "months" from datetime.c to from_char_seq_search() and
> calculates its length using "months_full" array from formatting.c.
>
With the introduction of seq_search_localized() that can be avoided,
minimizing code churn.
Please, find attached a version addressing the above mentioned.
Regards,
Juan José Santamaría Flecha
>
>
0001-Allow-localized-month-names-to_date-v6.patch
Description: Binary data
