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

Markdown extensions to support Math, Tables, etc. not working #936

Open
3 tasks done
mgc8 opened this issue Nov 22, 2022 · 24 comments
Open
3 tasks done

Markdown extensions to support Math, Tables, etc. not working #936

mgc8 opened this issue Nov 22, 2022 · 24 comments
Labels
bug needs-contributor Someone needs to implement this. Help wanted!
Milestone

Comments

@mgc8
Copy link
Contributor

mgc8 commented Nov 22, 2022

Checklist

  • I am running the latest version. Installing Isso from GitHub from the master branch does not fix my issue
  • I have checked the troubleshooting guide
  • I have searched the open issues, but my issue has not already been reported

What is not working?

The documentation here makes note of a number of extensions that can be added to the default Markdown editor, such as "math", "footnotes", "highlight", "tables" etc. However, even after adding them to the configuration, there does not seem to be any change, and no errors in the logs either.

How can one reproduce this issue?

Isso configuration file contains the following [markup] section:

[markup]
options = autolink, fenced-code, no-intra-emphasis, strikethrough, superscript, math, highlight, tables, underline
flags = hard-wrap
allowed-elements =
allowed-attributes =

The "default" ones -- "autolink, fenced-code, no-intra-emphasis, strikethrough, superscript" work correctly and as expected, adding and removing them from the line produces the expected results. However, "math, highlight, tables, underline" produce no effect whatsoever. A demo can be seen on this page, the last comment:
https://mihnea.net/hello-world/#isso-11

Is there something I'm missing from the installation? The Misaka library appears to be present under isso/lib/python3.9/site-packages/misaka and is at version 2.1.1.

@ix5 ix5 added needs-contributor Someone needs to implement this. Help wanted! bug labels Nov 24, 2022
@ix5 ix5 added this to the 0.14 milestone Nov 24, 2022
@ix5
Copy link
Member

ix5 commented Jan 8, 2023

Hi, thanks for reporting your issue.

Could you please provide a working example of your config, the text you typed in and the rendered text you expected to see?

(Related: #751)

@mgc8
Copy link
Contributor Author

mgc8 commented Jan 9, 2023

Hi @ix5, thank you for looking into this.

Here is the configuration in use:

[general]
dbpath = <path>/example.com-comments.db
name = example.com
host =
    http://example.com/
    https://example.com/
max-age = 15m
notify = smtp
reply-notifications = true
log-file = <path-to-isso>/sites/example.com.log
gravatar = true
gravatar-url = https://www.gravatar.com/avatar/{}?d=monsterid&s=55&r=x
latest-enabled = false

[admin]
enabled = false
password = please_choose_a_strong_password

[moderation]
enabled = true
approve-if-email-previously-approved = false
purge-after = 30d

[server]
listen = http://localhost:1234
public-endpoint = https://example.com/isso
reload = false
profile = false
trusted-proxies = 127.0.0.1
samesite = Strict

[smtp]
username =
password =
host = localhost
port = 25
security = none
to = webmaster
from = "example.com Comments" <webmaster@example.com>
timeout = 10

[guard]
enabled = true
ratelimit = 2
direct-reply = 3
reply-to-self = false
require-author = true
require-email = true

[markup]
options = autolink, fenced-code, no-intra-emphasis, strikethrough, superscript, math, highlight, tables, underline
flags = hard-wrap
allowed-elements =
allowed-attributes =

[hash]
salt = <salt>
algorithm = pbkdf2

[rss]
base =
limit = 100

You can see the examples in the link I posted, but sure I can add them here as well for clarity:

  1. This code in the comment _underline_ **bold** ~~strikethrough~~ *emphasis* ==highlight==
    => produces this HTML: underline <strong>bold</strong> <del>strikethrough</del> <em>emphasis</em> highlight

You can see that 'bold/strike/emphasis' have tags added, but the other two 'underline/highlight' don't, despite the extensions being specified in the config

  1. The 'math' extension should parse LaTeX-style code, right? This does not do anything at all (I took the example from https://github.blog/2022-05-19-math-support-in-markdown/ )
    When $a \ne 0$, there are two solutions to $(ax^2 + bx + c = 0)$ and they are $$ x = {-b \pm \sqrt{b^2-4ac} \over 2a} $$

  2. Finally, the 'tables' extension should parse code like the following, but doesn't appear to do anything either:

| Tables   |      Are      |  Cool |
|----------|:-------------:|------:|
| col 1 is |  left-aligned | $1600 |
| col 2 is |    centered   |   $12 |
| col 3 is | right-aligned |    $1 |

Hope this helps tracking things down, if I can provide more information just let me know.

@joseph-vidal-rosset
Copy link

joseph-vidal-rosset commented Mar 5, 2024

Hello. Thanks for isso, that's a very nice comments engine.
@ix5 , I ask exactly the same question that was @mgc8's. Is math option works with isso? Do we need to install something more in order to get equations inside comments? (Same questions for tables and all options that does not work as it was remarked in this thread.) Thanks for your help.

@joseph-vidal-rosset
Copy link

Markdown on Github explains that it works ```math
For example
The Cauchy-Schwarz Inequality

$$\left( \sum_{k=1}^n a_k b_k \right)^2 \leq \left( \sum_{k=1}^n a_k^2 \right) \left( \sum_{k=1}^n b_k^2 \right)$$

but also with '$$' :

$$ \left( \sum_{k=1}^n a_k b_k \right)^2 \leq \left( \sum_{k=1}^n a_k^2 \right) \left( \sum_{k=1}^n b_k^2 \right) $$

Therefore it should works with isso too.

@ix5
Copy link
Member

ix5 commented Apr 23, 2024

Hi @joseph-vidal-rosset, sorry for the late response. Isso's markdown parser is misaka, which needs some configuration to allow more advanced syntax.

As for LaTeX: You need something to actually render in the browser. misaka simply ignores/doesn't modify anything between $ / $$ and leaves it up to the browser to style it. This differs from GitHub who "magically" do the right thing for you.

https://docs.mathjax.org/en/v2.7-latest/start.html#mathjax-cdn suggests e.g.:

<script type="text/javascript" async
  src="https://cdnjs.cloudflare.com/ajax/libs/mathjax/2.7.7/MathJax.js?config=TeX-MML-AM_CHTML">
</script>

We as the Isso project would do well to make the handling of markdown parsing more intuitive and automatic as well as align it with GHFM. I have tried documenting the current behavior here: docs: Configure how comments are rendered.

@ix5
Copy link
Member

ix5 commented Apr 23, 2024

As for underline not working: You need to allow the <u> element in [markup] allowed-elements:

[markup]
options = [...], underline
[...]
allowed-elements = u

As for your table not displaying: That's unfortunately a similar issue, you need to allow the tr HTML tag in allowed-elements. Another contributor is working on simplifying this a bit, see #1007

We plan on doing the right thing for you when you enable a markdown extension automatically (e.g. allow all table-related elements when you use the tables extension).

@mgc8
Copy link
Contributor Author

mgc8 commented Apr 24, 2024

As for underline not working: You need to allow the <u> element in [markup] allowed-elements:
(...)
As for your table not displaying: That's unfortunately a similar issue, you need to allow the tr HTML tag in allowed-elements. Another contributor is working on simplifying this a bit, see #1007

Hello @ix5 , thank you for the additional information, we finally have a direction to explore on fixing this.

Using your suggestion, I was able to get "underline" to work via the "u" tag, but nothing else. "highlight" still does nothing (what would be the tag for that?), while the "table" also does absolutely nothing. Is this the correct syntax for the allowed-elements option?

allowed-elements = u, tr

I tried adding all the table-related tags there as well, with no difference. Really not sure why tr specifically had been blocked before, but in any case -- what else is needed to get this to work?

We plan on doing the right thing for you when you enable a markdown extension automatically (e.g. allow all table-related elements when you use the tables extension).

That would be great indeed, but until that's in place it would be great to be able to get these extensions working even manually, since right now the documentation you point to at docs: Configure how comments are rendered is a bit misleading since most extensions don't do anything...

@mgc8
Copy link
Contributor Author

mgc8 commented Apr 24, 2024

Hi @joseph-vidal-rosset, sorry for the late response. Isso's markdown parser is misaka, which needs some configuration to allow more advanced syntax.
As for LaTeX: You need something to actually render in the browser. misaka simply ignores/doesn't modify anything between $ / $$ and leaves it up to the browser to style it. This differs from GitHub who "magically" do the right thing for you.

Hmm, ok, but that's a third-party JavaScript library with no connection to misaka discussed here. Why is there a math extension specifically, if it does absolutely nothing? The documentation you linked to specifically says:
"Parse inline LaTeX-style math blocks, such as inline $equations$ or display $$equations$$ >> Extension name: math"

What kind of "parsing" is meant here?

I don't think it's a problem if this functionality is not actually supported, but in that case we should fix the documentation so that people don't waste time troubleshooting something that was never intended to work...

@ix5
Copy link
Member

ix5 commented Apr 25, 2024

"highlight" still does nothing (what would be the tag for that?)

That would be the <mark> element

while the "table" also does absolutely nothing

For me, [markup] options = no-intra-emphasis, strikethrough, underline, highlight, tables, math with allowed-elements = u, tr, mark works with the following table:

| Tables   |      Are      |  Cool |
|----------|:-------------:|------:|
| col 1 is |  left-aligned | $1600 |
| col 2 is |    centered   |   $12 |
| col 3 is | right-aligned |    $1 |

As well as with _underline_ **bold** ~~strikethrough~~ *emphasis* ==highlight==

since right now the documentation you point to at docs: Configure how comments are rendered is a bit misleading since most extensions don't do anything...

Yes, the issue is known. We have to decide just how much we want to overhaul the whole system without breaking setups of existing users.

Hmm, ok, but that's a third-party JavaScript library with no connection to misaka discussed here. Why is there a math extension specifically, if it does absolutely nothing? The documentation you linked to specifically says:
"Parse inline LaTeX-style math blocks, such as inline $equations$ or display $$equations$$ >> Extension name: math"

That's due to the underlying misaka library choices: FSX/misaka#71 (comment)

we should fix the documentation so that people don't waste time troubleshooting something that was never intended to work...

Feel free to submit a PR

@mgc8
Copy link
Contributor Author

mgc8 commented Apr 26, 2024

Hi @ix5 ,

Thank you for the clarifications, this was very helpful to finally fix and understand the behaviour here!

"highlight" still does nothing (what would be the tag for that?)
That would be the <mark> element

Done, works fine, thanks!

while the "table" also does absolutely nothing
For me, [markup] options = no-intra-emphasis, strikethrough, underline, highlight, tables, math with allowed-elements = u, tr, mark works with the following table:
(...)

Interestingly, I had to track this for a bit. It seemed to work fine in a new comment, but not in the existing one(s). What eventually turned out to be the problem was that the rendering did not kick in if there was any text right next to the table, it would keep it as 'code' then. Adding a line break made it render fine!

As well as with _underline_ **bold** ~~strikethrough~~ *emphasis* ==highlight==

Yes, all of these work now after adding the missing tags under allowed-elements.

That's due to the underlying misaka library choices: FSX/misaka#71 (comment)

This was pretty unclear to me until spending some time to read the information in that comment. Basically, the math extension doesn't actually do anything, but it marks the LaTeX-style code so that it is skipped by misaka, allowing other (external, third-party, etc.) libraries to parse and render it later. Now I understand.

we should fix the documentation so that people don't waste time troubleshooting something that was never intended to work...

Feel free to submit a PR

Ok, done -- #1017 -- I've added the above few notes, hope it's ok!

@joseph-vidal-rosset
Copy link

I am sorry, but for me the situation stays the same: neither ==highlight== nor table nor math formula are rendered in isso comments...

@mgc8
Copy link
Contributor Author

mgc8 commented Apr 26, 2024

I am sorry, but for me the situation stays the same: neither ==highlight== nor table nor math formula are rendered in isso comments...

Maybe if you paste the contents of your [markup] section in the configuration file would help trace what's happening... You do have the allowed-elements = u, tr, mark line, right? And you've restarted uwsgi or the specific WSGI tool you use to run isso?

@joseph-vidal-rosset
Copy link

Sorry, I did not restart isso, and it works now, for _underline_ **bold** ~~strikethrough~~ *emphasis* ==highlight== as well as for tables. Thanks !
But, unfortunately, for $$ A \to B $$ the result remains the same: it does not work. Did I miss something ?

@mgc8
Copy link
Contributor Author

mgc8 commented Apr 27, 2024

Sorry, I did not restart isso, and it works now, for _underline_ **bold** ~~strikethrough~~ *emphasis* ==highlight== as well as for tables. Thanks !

Super, great to hear, also to have that note here in case other people run into the same issue!

But, unfortunately, for $$ A \to B $$ the result remains the same: it does not work. Did I miss something ?

Yes, that is actually expected behaviour, as detailed by @ix5 above. I have a PR attempting to improve the documentation around this -- basically the math extension doesn't actually do anything, it just prevents misaka (or Isso in this case) from interpreting the LaTeX code as Markdown. Actually rendering it as proper equations/formulas is left for a third-party library, they recommend MathJax or KaTeX for that.

I've researched those a bit (both are JavaScript based and also require a consistent amount of custom fonts), and each have some pros and cons: MathJax is a much smaller library (14k or so) and with better compatibility for ancient browsers (we're talking really ancient), but it's also very slow to render and suffers from a nasty FOUC effect (Flash Of Unstyled Content); KaTeX is a significantly "heavier" library (over 200k minified), but it's more modern and lightning fast, avoiding FOUC altogether. Remains up to everyone to choose the best one for their use case (I'd go with KaTeX personally).

Interestingly, there's also the option of parsing the LaTeX server-side: KaTeX for example provides this (but being written in JS, it means you'd need to have an entire NodeJS infrastructure on the server); but there are also Python implementations for this -- for example, mdtex2html, which I've tested on the above equations and comes up very nicely in MathML (supported in all modern browsers without extra fonts or JS necessary):
Screenshot 2024-04-27 at 20 17 40

@ix5 -- would you be interested in pursuing a way to integrate this, or perhaps latex2html directly, into Isso? It could be run only if math is detected in the comments, and would avoid the need for external and heavy libraries and post-processing. Happy to study options if you find it can fit the scope.

@joseph-vidal-rosset
Copy link

MathJax is very efficient and it is used in my blog. Unfortunately, it is not used in isso. The best, in my opinion, would be the option of using it in comments.

@mgc8
Copy link
Contributor Author

mgc8 commented Apr 28, 2024

MathJax is very efficient and it is used in my blog. Unfortunately, it is not used in isso. The best, in my opinion, would be the option of using it in comments.

If you already have MathJax running on your blog, then it should just be a matter of getting it to parse the comments after Isso. You could either delay its loading and running for until after Isso is done (moving the script to the footer and removing any async tags, for example), or add a bit of JS to specifically run MathJax on the newly created divs. Maybe the MathJax documentation has some help in this regard?

@ix5
Copy link
Member

ix5 commented Apr 29, 2024

Regarding @mgc8's suggestion about integrating a latex/mathml conversion tool: It would be nice to hear from existing users whether they would be interested in that or if their own control over math rendering is more important.

We should also figure out the order of the following steps:

  • render markdown (misaka)
  • clean HTML (bleach)
  • convert latex to mathml (essentially <math> HTML elements)

I'm not sure whether we could introduce vulnerabilities if we do the latex conversion after the bleach step since perhaps users could do $a gt b <script>evil</script>$? Is that properly guarded against?

@joseph-vidal-rosset
Copy link

MathJax is very efficient and it is used in my blog. Unfortunately, it is not used in isso. The best, in my opinion, would be the option of using it in comments.

If you already have MathJax running on your blog, then it should just be a matter of getting it to parse the comments after Isso. You could either delay its loading and running for until after Isso is done (moving the script to the footer and removing any async tags, for example), or add a bit of JS to specifically run MathJax on the newly created divs. Maybe the MathJax documentation has some help in this regard?

You are very probably right. I have to look again on MathJax 3 Documentation, how to use MathJax on dynamic contents.

@joseph-vidal-rosset
Copy link

I suggest a help in isso-documentation to use MathJax with isso...

@mgc8
Copy link
Contributor Author

mgc8 commented Apr 29, 2024

Regarding @mgc8's suggestion about integrating a latex/mathml conversion tool: It would be nice to hear from existing users whether they would be interested in that or if their own control over math rendering is more important.

That makes sense, although I'm not sure how many users read this issue... How about this: we can have the server-side parsing controlled by an option, which would default to the current behaviour if missing or not set, thus not affecting any existing setup? Then, only the users who really want to have that would turn it on.

We should also figure out the order of the following steps:

  • render markdown (misaka)
  • clean HTML (bleach)
  • convert latex to mathml (essentially <math> HTML elements)
    I'm not sure whether we could introduce vulnerabilities if we do the latex conversion after the bleach step since perhaps users could do $a gt b <script>evil</script>$? Is that properly guarded against?

That is a very good point, security is critical in a case like this. I would say that bleach should ideally be run as the last step, in order to sanitise the potential output of latex2mathml as well, though a cursory test shows that none of the MathML tags are allowed by default, so we would need to pass a comprehensive tags list to the bleach.clean() call. This could also be controlled by the previously mentioned option, so the tags would be allowed if and only if the user had server-side math parsing enabled, which should minimise the potential attack surface.

Alternatively, we can also run the latex2mathml conversion last, which appears to be quite resilient to script/tag injection since it treats those as LaTeX anyway -- e.g., your example above would be translated as the following harmless MathML:

<math xmlns="http://www.w3.org/1998/Math/MathML" display="inline"><mrow><mi>a</mi><mo>&#x0003E;</mo><mi>b</mi><mo>&#x0003C;</mo><mi>s</mi><mi>c</mi><mi>r</mi><mi>i</mi><mi>p</mi><mi>t</mi><mo>&#x0003E;</mo><mi>e</mi><mi>v</mi><mi>i</mi><mi>l</mi><mo>&#x0003C;</mo><mo>&#x0002F;</mo><mi>s</mi><mi>c</mi><mi>r</mi><mi>i</mi><mi>p</mi><mi>t</mi><mo>&#x0003E;</mo></mrow></math>

... though that would still potentially leave some malicious manipulation possible to output HTML; although nothing is perfect, bleach itself seems to be at a bit on impasse right now according to mozilla/bleach#698

@ix5
Copy link
Member

ix5 commented Apr 29, 2024

How about this: we can have the server-side parsing controlled by an option, which would default to the current behaviour if missing or not set, thus not affecting any existing setup? Then, only the users who really want to have that would turn it on.

Thus further complicating our already byzantine maze of configuration options into which you've already felt yourself getting trapped...

bleach itself seems to be at a bit on impasse right now according to mozilla/bleach#698

That's certainly an issue, thank you for raising that (I wasn't aware). People suggest using nh3 so a switch in the future should be investigated.


In general, math rendering on the server makes sense, since hooking the mathjax/... calls, especially for the edit/preview fields, is a pain. We currently provide mostly barebones functionality and trust our mostly-technical users to DIY the rest.

If we find a way to make this less painful while keeping Isso simple enough, I'm all for that.

@mgc8
Copy link
Contributor Author

mgc8 commented Apr 29, 2024

Thus further complicating our already byzantine maze of configuration options into which you've already felt yourself getting trapped...

To be honest, I don't think the confusion here stemmed from the configuration options, but rather the unintuitive behaviour of the software behind-the-scenes. Overall, Isso has a pretty succint configuration compared to other similar pieces of software.

With the recent patches that add some more sanity to the way allowed-elements & co. function, I think that will be much less of a problem. I followed your notes on the other issues related to that, and I do agree that the overall Markdown parsing infrastructure in Isso could be improved, but I think having this option here would actually offer a valuable choice to end-users between server-side and client-side parsing (at least for one particular feature).

Of course, if you have already something else in mind as part of future plans for improvement here, let me know and perhaps we can aim for that instead?

bleach itself seems to be at a bit on impasse right now according to mozilla/bleach#698
That's certainly an issue, thank you for raising that (I wasn't aware). People suggest using nh3 so a switch in the future should be investigated.

From what I see in the (many) comments on that, nh3 appears to be much faster (although, probably not very relevant for Isso since comments are typically short), but it does come with certain incompatibilities/strictness errors which have caught some people unawares; we should probably have a good testing suite in place before making the switch. On the other hand, bleach developers appear committed to at least supporting the project with security and Python version updates, so it's not dire yet...

In general, math rendering on the server makes sense, since hooking the mathjax/... calls, especially for the edit/preview fields, is a pain. We currently provide mostly barebones functionality and trust our mostly-technical users to DIY the rest.
If we find a way to make this less painful while keeping Isso simple enough, I'm all for that.

Agreed on all counts, if other people have comments and suggestions on the topic, that would be great to hear.

@ix5
Copy link
Member

ix5 commented May 5, 2024

To be honest, I don't think the confusion here stemmed from the configuration options, but rather the unintuitive behaviour of the software behind-the-scenes. Overall, Isso has a pretty succint configuration compared to other similar pieces of software.

With the recent patches that add some more sanity to the way allowed-elements & co. function, I think that will be much less of a problem. I followed your notes on the other issues related to that, and I do agree that the overall Markdown parsing infrastructure in Isso could be improved, but I think having this option here would actually offer a valuable choice to end-users between server-side and client-side parsing (at least for one particular feature).

You're right in that the markdown handling will be improving a bit, but that may be a bit for naught if we are to change our renderer. I would prefer to expose sensible options to users rather than have them learn the intricacies of our renderer/sanitizer, which might not translate 1-to-1 for the different packages (mistune/... and nh3/...) we might have to replace them with due to deprecations.

We can lessen the confusion of the current markdown configuration via better documentation and improvements to auto-enabling allowed HTML elements when toggling a feature flag, but it'd be much better for existing and especially new users to just make this whole system intuitive.

I'd prefer to make the defaults more like GitHub-flavored markdown (meaning also enabling more extensions by default) and expose extensions to users via generic names (images, tables, links) instead of being tied to misaka's naming.

But that's just my own opinion and I'm willing to be convinced otherwise. Discussing this here is a good first step towards finding a solution.

My idea would be something like this:

[markdown]
line-break = single # hard-warp
raw-html = false # escape

[markdown.features]
tables = true
latex-math = render # or 'skip', 'escape'
images = false
links = true
# Enabled by default
highlight = true
underline = true
strikethrough = true
no-intra-emphasis = true
# ...

As for latex/math handling on the server vs client (via JS), the client isn't really ready for that as I see it. We do not provide a reliable hook mechanism for newly inserted comments (or even the preview field) so that Mathjax could listen for that - and unless this sorry state of affairs changes, I'd rather simply do the right thing for them on the server. Letting users know that something is here-be-dragons territory always sucks. Let's improve the client and expose extensibility mechanisms and a whole lot of problems suddenly get easier.

From what I see in the (many) comments on that, nh3 appears to be much faster (although, probably not very relevant for Isso since comments are typically short), but it does come with certain incompatibilities/strictness errors which have caught some people unawares; we should probably have a good testing suite in place before making the switch. On the other hand, bleach developers appear committed to at least supporting the project with security and Python version updates, so it's not dire yet...

Good to know.

In general, math rendering on the server makes sense, since hooking the mathjax/... calls, especially for the edit/preview fields, is a pain. We currently provide mostly barebones functionality and trust our mostly-technical users to DIY the rest.
If we find a way to make this less painful while keeping Isso simple enough, I'm all for that.

Agreed on all counts, if other people have comments and suggestions on the topic, that would be great to hear.

[...]

Of course, if you have already something else in mind as part of future plans for improvement here, let me know and perhaps we can aim for that instead?

Perhaps a more focused approach with actionable items might lead to better and clearer results. Right know we're burying the conversation deep in this thread, which was originally a bug report (I'm absolutely guilty of perpetuating this as well).

Suggestions are always great, and we have lots of drive-by contributions with small to medium-sized fixes/improvements, but so far, no features/improvements of a larger scale (as would be required by the overhaul in markdown handling) have been contributed in recent times.

If you feel that you would like to overhaul this system or know someone for whom this would be just the right challenge, go for it! While I personally am neither a great coder nor could design a good system, I'd be happy to review any proposals and contribute docs, migration advisories (from old handling to new) and tests.

@joseph-vidal-rosset
Copy link

Using MathJax 3 dynamically in DOM is not quite easy. If someone would succeed to use MathJax in isso comments, an explanation online would be very welcome....

@ix5 ix5 modified the milestones: 0.14, 0.13.1 May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug needs-contributor Someone needs to implement this. Help wanted!
Projects
None yet
Development

No branches or pull requests

3 participants