Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not localize dates that appear within code, hyperlinks and quote blocks #48

Closed
wants to merge 11 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Jan 5, 2022

Summary

Currently this plugin localizes dates within the full raw message text (that may include Markdown) as sent by a user.

Dates that appear within code blocks should not be localized, since they represent a form of quoted context that should be rendered as-is to recipients. The same is true for blockquotes.

The same is true of hyperlinks, as reported in #49.

To detect and ignore those elements, this change uses the Mattermost-flavoured fork of the marked Markdown processor (as used by mattermost-webapp) to filter them out from the text that is scanned for localizable dates.

Ticket Link

Resolves #25.
Resolves #49.

@mattermod
Copy link
Contributor

Hello @jayaddison-collabora,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

@ghost

This comment was marked as outdated.

@ghost ghost changed the title Do not localize dates that appear within code spans Do not localize dates that appear within code spans and hyperlinks Jan 5, 2022
@ghost

This comment has been minimized.

const filterRenderer = new marked.Renderer();
filterRenderer.code = () => '';
filterRenderer.codespan = () => '';
filterRenderer.link = () => '';
Copy link
Author

Choose a reason for hiding this comment

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

It looks like there may be other cases that would make sense to ignore too; blockquote for example. Perhaps it'd make sense to create an array of the renderer properties to override and assign to each of them in a loop.

@ghost ghost changed the title Do not localize dates that appear within code spans and hyperlinks Do not localize dates that appear within code and hyperlinks Jan 5, 2022
@ghost ghost changed the title Do not localize dates that appear within code and hyperlinks Do not localize dates that appear within code, hyperlinks and quote blocks Jan 20, 2022
@ghost
Copy link
Author

ghost commented Jan 20, 2022

@mkraft apologies for the repeat notifications; here's another, slightly more involved, merge request.

We deployed a version of mattermost-plugin-walltime at @collabora on 2020-01-05 and I'm yet to hear any reports of complaints / problems with time localization problems since then (that's not to say it hasn't happened, but I'm not yet aware of any problems related to the plugin deployment since then).

The code from that deployment is visible at https://gitlab.collabora.com/tools/mattermost/mattermost-plugin-walltime/-/commits/deploys/test-20220105 -- it includes most of the changes here albeit without the hyperlink and code block filtering.

@ghost
Copy link
Author

ghost commented Jan 27, 2022

Please note that the merge commit here (7b11ab0) involved some manual conflict resolution regarding the NPM package-lock.json file.

It wasn't possible to (easily) have the npm command-line tool resolve these automatically, because the version of the CLI tool I ran locally (v8.3.1) complained about an integrity hash change for the marked package dependency.

That's potentially related to some kind of change that NPM made regarding the calculation of integrity checksums. There's a possible note about that here (I'm not really completely clear on the root cause of this, but have seen the same problem occur within other projects as well).

@mattermod
Copy link
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

/cc @aspleenic

@ghost
Copy link
Author

ghost commented Mar 11, 2022

cc @mkraft @cpanato (apologies for the notification spam; this is the last of the pull requests I'll mention you on for now) - this is a change to prevent text within preformatted markdown blocks from being transformed by the mattermost-plugin-walltime plugin.

@ghost
Copy link
Author

ghost commented Mar 18, 2022

A personal update: I'm planning to resign from @collabora effective Friday 2022-04-01, so this pull request may go unmaintained from that point.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Time is converted in hyperlinks Time is converted in preformatted text
1 participant