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

Pass user to parse and request to render #35

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

n-peugnet
Copy link

@n-peugnet n-peugnet commented Feb 9, 2023

Some extensions like flarum-mentions or club-1-cross-references make use of these additional parameters. So we need to pass these to able to render the messages.

See parse and render signatures from the API docs : https://api.docs.flarum.org/php/v1.6.0/flarum/formatter/formatter

Fixes #34

@n-peugnet
Copy link
Author

n-peugnet commented Feb 9, 2023

You should then bump your flarum/core version to ^1.6.0 in composer.json as the third parameter of parse() is only present since this commit : flarum/flarum-core@f54bd6f

(The request parameter of render() is there since 1.3.0 : flarum/flarum-core@c3d9f43)

I didn't do it in this commit as the changes in composer.json and composer.lock would have added a lot of noise in the diff.

n-peugnet added a commit to club-1/flarum-ext-cross-references that referenced this pull request Feb 12, 2023
As it seems a lot of extension do not set the parameter when calling
Formatter->render(), we fallback to unknown discussion rendering when
request is null.

We keep logging an error, as this issue should be solved in these
extensions.

See also <the-turk/flarum-diff#35> for an
example of how to fix this issue.
@n-peugnet
Copy link
Author

Hmm this is strange. I am sure that it worked, but this does not seem to work anymore. I'm investigating.

Some extensions like flarum-mentions or club-1-cross-references make use
of these additionnal parameters. So we need to pass these to able to
render the messages
@n-peugnet n-peugnet force-pushed the fix-parse-and-render-missing-args branch from 5b0bb2a to e2a1058 Compare February 12, 2023 21:37
@n-peugnet
Copy link
Author

Okay. I just pushed a new commit to pass the correct actor to parse() for each revision.

@n0099
Copy link

n0099 commented Mar 5, 2023

LGTM
related: s9e/TextFormatter#214

@rafaucau
Copy link

rafaucau commented Apr 9, 2023

@the-turk Could you please merge?

@n0099
Copy link

n0099 commented Apr 9, 2023

@the-turk Could you please merge?

You can apply changes in this pr without waiting for @the-turk to merge it and bump a new version on packagist by switching the source of package the-turk/flarum-diff from packagist to branch club-1/flarum-diff/fix-parse-and-render-missing-args in composer.json:
https://getcomposer.org/doc/05-repositories.md
https://stackoverflow.com/questions/12954051/use-php-composer-to-clone-git-repo

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.

Error 500 when trying to display revisions of a post that contains an mention to a post (reply)
3 participants