--redirect: fix check for number of columns in redirect file

[haoess]

No description provided.

[kelson42]

Not sure exactly what are the rationals behind both bug and fix. But this probably desserves a better automated testing.

[rgaudin]

Here's a valid redirect file

 \tAccueil\tBienvenue !!\twelcome\n
 \tAccueil2\t\tcommons48.png\n
 \timage\t\tcommons48.png\n

Notice that it starts with a space and a tab \t. That space was used by the (now unused) namespace.

We have support for it in scraperlib here with a test

As per the doc, size() returns number of marked subexpressions plus 1

blame clearly shows that @mgautierfr changed the regexp 3y ago to remove the namespace but did not adjust the match count. scraperlib's implementation precedes the change.

[mgautierfr]

Thanks @rgaudin for the investigation.

Here's a valid redirect file [...]

Is it really a valid file ? The regex search for 2 \t (3 fields) but you have 3 \t.

@haoess Please remove the merge commit of your own PR in your own main.

@haoess It would be nice to have a testing of that code. Can you create it ? (It would probably need some refactoring of the code to separate the parsing of the file from adding the redirect)

[haoess]

@haoess Please remove the merge commit of your own PR in your own main.

(Hopefully) done.

@haoess It would be nice to have a testing of that code. Can you create it ?

Sorry, I’m not a C++ dev. Applying some knowledge about how other languages handle captures in RE, using a pattern with three pairs of parens (…) and testing the number of captures (+ 1 for the whole match) against 5 is not supposed to work (= what @rgaudin says).

[kelson42]

@mgautierfr please don't merge without automated test.

[mgautierfr]

(Hopefully) done.

No, I doesn't. The merge commit was the commit merging the change in your main. By reverting the merge commit, you removing the change. That's why we have 0 file changed.

Anyway, I have create #360 which takes your commit and add the tests.
So I'm closing this PR and we will continue on #360.
Thanks for your fix anyway.