Premising I very like the strategy in this patch that is "don't parse unuseful stuff" in order to "don't suppress weird errors", so, causing to preserve the user input.

I'm trying to work on a way to keep the possibility to implement whatever generic "interpreter" but without altering the user input just because we have not found any weird "interpreter". For these cases, logging could be really enough.

I discussed with aklapper and, hoping to be useful to save the time of this precious contributor, I will update with the patch that we have seen together in live during Wikimedia Hackaton (so you don't need to amend and etc.)

In D25234#7296, @avivey wrote:

I don't see how this stops the exception discussed in T15372 - nothing in this change looks like it might stop an "Undefined array key" exception?

I need some time to also clarify that specific question, but I can reproduce the current issue with this Comment:

Ciccio {{{Foo}}} indeed

Try, and you will not be able to save a new Comment. With the proposed patch, it does not happen anymore.

Ah yeah indeed now I understand the issue:

The parser was matching some line(s), but eventually returning an error message with potentially a different number of line (1). So, now it seems this also matches the expected lines since I cannot reproduce that problem anymore.

Where was it "returning an error message with potentially a different number of line" ?

In D25234#7303, @avivey wrote:

Where was it "returning an error message with potentially a different number of line" ?

As far as I've understood, line 81 was returning the error $message that has a fixed length ("No interpreter found", 1 line) but that was not handled correctly since it's length was not matching the one returned from getMatchingLineCount(), that does not take into consideration potential errors.

So, with this patch we do not need anymore to fix the getMatchingLineCount() method, since now it is more consistent with the returned lines. This should be now true with all cases, not just non-erroneus cases.

That sounds very sketchy - getMatchingLineCount should either refer to raw text, or to post-processed text - and the 2nd option isn't possible (There might not be any text in the result, like in macro).

So I would expect the output of getMatchingLineCount not to depend on the output of markupText at all - definitely not force the implementation to have a specific "line count".

Just to clarify: I think we both agree on these four things:

1. showing a "No interpreter error" to the frontend could be weird, at minimum this is true for MediaWiki users (not just Wikimedia users) and showing the original raw text could be more useful (green light)
2. after this ↑ it has sense to just log the problem (green light)
3. this Diff works and also "somehow" fixes an error (green light)
4. it could be really interesting to discover where was the root problem (→ let's dig)

With these premises, I totally agree that the n. 4 is scarying. It seems the business logic reading getMatchingLineCount() is inside PhutilRemarkupEngine::splitTextIntoBlocks().

I would really thank you for whatever additional note you eventually discover. I will do the same.

In D25234#7306, @valerio.bozzolan wrote:

Just to clarify: I think we both agree on these four things:

1. showing a "No interpreter error" to the frontend could be weird, at minimum this is true for MediaWiki users (not just Wikimedia users) and showing the original raw text could be more useful (green light)

I'm not sure I agree with this. This case is strange enough that I think anyone who got here, got here by trying to use this feature, not by mistake. In that case, showing an error is clearer then just passing the input as is (and not interpreting whatever remarkup it has in the body!)

2. after this ↑ it has sense to just log the problem (green light)

I don't agree with this - as I said, this is a user error, and logs are generally only shown to administrators.

2. this Diff works and also "somehow" fixes an error (green light)

I'm not sure it "fixes" the problem or just "hides" it - that depends on the definition of the problem.

4. it could be really interesting to discover where was the root problem (→ let's dig)