Skip to content

remove unnecessarry comma from cosmic-applet-time#1393

Open
Lucas-BRT wants to merge 1 commit into
pop-os:masterfrom
Lucas-BRT:fix/pt-BR-unnecessary-comma
Open

remove unnecessarry comma from cosmic-applet-time#1393
Lucas-BRT wants to merge 1 commit into
pop-os:masterfrom
Lucas-BRT:fix/pt-BR-unnecessary-comma

Conversation

@Lucas-BRT
Copy link
Copy Markdown

  • I have disclosed use of any AI generated code in my commit messages.
    • If you are using an LLM, and do not fully understand the changes it is making to the code base, do not create a PR.
    • In our experience, AI generated code often results in overly complex code that lacks enough context for a proper fix or feature inclusion. This results in considerably longer code reviews. Due to this, AI authored or partially authored PRs may be closed without comment.
  • I understand these changes in full and will be able to respond to review comments.
  • My change is accurately described in the commit message.
  • My contribution is tested and working as described.
  • I have read the Developer Certificate of Origin and certify my contribution under its conditions.

When the system locale is set to pt-BR, the date displayed in the top or bottom panel includes a comma between the abbreviated month and the time, resulting in two consecutive punctuation marks: 25 de abr., 09:35. Image

The vertical layout is not affected because it formats date and time separately.

The horizontal layout, however, combines them via ICU4X's datetime combining pattern for pt-BR, which appears to be "{1}, {0}" (comma-separated) instead of "{1} {0}".

The . is correct, it belongs to the abbreviated month (abr.).

The problem is the , that ICU4X inserts as a separator between the date and time parts, producing the awkward ., sequence which is not natural in Brazilian Portuguese.

Steps to reproduce

  1. Set system locale to pt-BR
  2. Set Panel Position on screen to Bottom or Top

Expected behavior

25 de abr. 09:35 with no comma between date and time.

Proposed fix

Strip the combining comma from the formatted string for pt-BR, or format date
and time separately and concatenate with a space, similar to what the vertical
layout already does.

Image

I only need to know if in some other language this comma is necessary by some reason.

@Lucas-BRT Lucas-BRT changed the title []remove unnecessarry comma from cosmic-applet-time remove unnecessarry comma from cosmic-applet-time Apr 25, 2026
@jacobgkau jacobgkau requested a review from a team May 13, 2026 19:31
Copy link
Copy Markdown
Member

@jacobgkau jacobgkau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only need to know if in some other language this comma is necessary by some reason.

It's necessary for English, so we can't accept the PR in its current form. This is the before/after:

Screenshot_2026-05-13_13-30-50 Screenshot_2026-05-13_13-30-59

Removing the comma leaves numbers separated by no punctuation, which we generally need to avoid. Any change to the current comma would need to be approved by the UX team.

A fix specific to your language could potentially be accepted. It seems weird we'd use an internationalization formatter and then have to touch up the result, though. Is it possible this is something that could be improved upstream in ICU4X instead of specifically in this applet?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants