1480919 - Run codespell on Calendar

Status: RESOLVED FIXED

(Calendar :: General, enhancement)

Description

[Jorg K (CEST = GMT+2)]

It has an enormous amount of spelling mistakes, see attachment 8960495 [details] [diff] [review].

When is a good time to do this?

Comment 1

[[:MakeMyDay]]

Thanks for taking care of that, much apprechiated. For changes in ical.js, you would need to do a pull request on Github [1] since that is imported from upstream (where it consists of multiple files). You have also changes in strings in that patch, that might require and update of the string name. Can you split all changes to .properties or .dtd files to a separate patch?

Since it looks that this is a rerunable scripted correction, I think we should hold that back still for 1 to 2 months to make backporting patches for ESR more easy.

[1] https://github.com/mozilla-comm/ical.js/

Comment 2

[Jorg K (CEST = GMT+2)]

(In reply to [:MakeMyDay] from comment #1)
> You have also changes
> in strings in that patch, that might require and update of the string name.
I don't think fixing an obvious spelling mistake (mostly in comments, but also occur(r)ed, transfer(r)ed) needs a new string.

OK, I'll revisit this in two months.

Comment 3

[Jorg K (CEST = GMT+2)]

I needed a patch to land, so I grabbed three directories from the original patch. No .properties or .dtd files and no ical.js.

Comment 4

[Jorg K (CEST = GMT+2)]

Attachment: Fix spelliing in providers, resources, test

Totally boring.

Geoff, ical.js is just one file, right? calendar/base/modules/ical.js

And I guess we don't want to mess with libical either. Can you please take a look at the original patch in attachment 8960495 [details] [diff] [review] and tell me what not to touch.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/d1aa51f98737
fix typos in calendar/providers, resources, test using codespell. rs=typo-fix,comment-only

Comment 6

[Geoff Lankow (:darktrojan)]

Comment on attachment 9053043 [details] [diff] [review]
Fix spelliing in providers, resources, test

What a good job we have computers to teach us how to spell.

Comment 7

[Geoff Lankow (:darktrojan)]

(In reply to Jorg K (GMT+1) from comment #4)

Created attachment 9053043 [details] [diff] [review]
Fix spelliing in providers, resources, test

Totally boring.

Geoff, ical.js is just one file, right? calendar/base/modules/ical.js

Yes, that's right.

And I guess we don't want to mess with libical either.

As I understand it, our copy of libical is fair game for stuff like this. Our fork is ancient and I don't believe there are any plans to update it. Apparently not.

Can you please take a look at the original patch in attachment 8960495 [details] [diff] [review] and tell me what not to touch.

I had a quick look. Nothing jumped out at me other than the two exceptions already mentioned. And this, which really should be fixed:

-                  commmand="calendar_publish_calendar_command"
+                  command="calendar_publish_calendar_command"

Comment 8

[Jorg K (CEST = GMT+2)]

Attachment: Calendar/locales

Mostly comments, some typos in strings, surely translators don't need to know we fixed them.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/303c1d12c1f6
fix typos in calendar/locales using codespell. rs=typo-fix,comment-only

Comment 10

[Jorg K (CEST = GMT+2)]

Attachment: calendar/itip and calendar/lightning

Second last one, then there's only calendar/base left, that has the most code and mistakes. Note that I'm not running codespell again, this is the old patch rebased. So the last pass will be to check that no new errors have crept in.

Comment 11

[Jorg K (CEST = GMT+2)]

Comment on attachment 9053986 [details] [diff] [review]
calendar/itip and calendar/lightning

Review of attachment 9053986 [details] [diff] [review]:
-----------------------------------------------------------------

::: calendar/lightning/content/lightning-menus.xul
@@ -382,5 @@
>                    observes="calendar_import_command"/>
>          <menuitem id="ltnPublishCalendar"
>                    label="&calendar.publish.label;"
>                    accesskey="&calendar.publish.accesskey;"
> -                  commmand="calendar_publish_calendar_command"

So what did or didn't work here? Any idea?

Comment 12

[Geoff Lankow (:darktrojan)]

Well that's weird. It has a commmand attribute, but an overlay adds oncommand="goDoCommand('calendar_publish_calendar_command')".

No, that's not right. Yes it is, the oncommand attribute is copied from the <command> because of the observes attribute. XUL is hard. :-(

Comment 13

[Jorg K (CEST = GMT+2)]

Well, should the misspelled commmmmmand just be removed?

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/6483318f4089
fix typos in calendar/itip and calendar/lightning using codespell. r=darktrojan DONTBUILD

Comment 15

[Geoff Lankow (:darktrojan)]

No, just fix the spelling, which you have already.

Comment 16

[Jorg K (CEST = GMT+2)]

Attachment: 1480919-recheck-no-base.patch

Plenty of new errors in non-base files :-(

Comment 17

[Jorg K (CEST = GMT+2)]

Attachment: 1480919-cal-base.patch

That fixes everything apart from libical and ical.js.

Comment 18

[Geoff Lankow (:darktrojan)]

Comment on attachment 9054576 [details] [diff] [review]
1480919-recheck-no-base.patch

> dependendent

Hah! :-)

Comment 19

[Geoff Lankow (:darktrojan)]

Comment on attachment 9054578 [details] [diff] [review]
1480919-cal-base.patch

Review of attachment 9054578 [details] [diff] [review]:
-----------------------------------------------------------------

A few things still to fix in this one.

::: calendar/base/content/calendar-common-sets.xul
@@ +203,5 @@
>          </menupopup>
>        </menu>
>        <menuseparator id="calendar-menuseparator-before-delete"/>
>        <!-- the label and accesskey of the following menuitem is set during runtime,
> +           and depends on whether the item is a task or an event-->

Please add a space before the --> while you're here. I should ask you to turn all these comments in to proper sentences with capital letters and full stops, but that's not what you're here for.

::: calendar/base/content/calendar-dnd-listener.js
@@ +394,1 @@
>          // notification to partticipants if required

partticipants?

::: calendar/base/content/calendar-item-editing.js
@@ +752,5 @@
>              }
>              let attendee = getParticipant(oldItem);
>              if (attendee) {
>                  // skip this item if the partstat for the participant hasn't
> +                // changed. otherwise we would always perform updade operations

updade?

Did you run codespell again after making these changes?

::: calendar/base/content/dialogs/calendar-event-dialog-recurrence.js
@@ +165,1 @@
>              // in order to visit each minimonth in turn.

This still doesn't make sense. "iterate through all" perhaps? Or just cut out the middle and go for "Now visit each minimonth in turn."

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/fe9337e031b8
fix new typos in calendar/ except calendar/base using codespell. r=darktrojan

Comment 21

[Jorg K (CEST = GMT+2)]

Attachment: 1480919-cal-base.patch (v2)

Comment 22

[Jorg K (CEST = GMT+2)]

Calendar comments frequently/mostly are not English sentences starting with upper case and ending with a full stop, not even new comments being added, and they pass review.

I ran codespell again and it didn't pick up "updade" and "partticipants". So I fixed it manually together with " -->" and "iterate".

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/5005f4ee1063
fix typos in calendar/base using codespell. r=darktrojan