Check for duplicate sources in messages

[zachcmadsen]

With rounded line numbers, it's possible for messages to have duplicate sources. This checks for duplicate sources before updating messages. An alternative approach is deduplicating sources after all the messages are added/updated. I went with the "on-the-fly" approach since it's simpler.

The check scans all of the sources for a message. If performance is a concern, we could try other ways to check for duplicates.

Fixes #154

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[codecov-commenter]

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (fba1b0a) 90.74% compared to head (ba57994) 90.80%.

Files	Patch %	Lines
i18n-helpers/src/xgettext.rs	94.73%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #156      +/-   ##
==========================================
+ Coverage   90.74%   90.80%   +0.05%     
==========================================
  Files          11       11              
  Lines        2399     2436      +37     
  Branches     2399     2436      +37     
==========================================
+ Hits         2177     2212      +35     
  Misses        159      159              
- Partials       63       65       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kdarkhan]

I think checking for existence should be done per line rather than regular string contains. An example case where the proposed logic would fail will be if source contains entry some-dir/README.md and the new source is README.md.

[zachcmadsen]

That makes sense. I'll update the logic and add a test case for that scenario

[mgeisler]

Good catch @kdarkhan! I think this could be handled nicely by

1. Separating the sources with \n when add_message is called.
2. Use lines() to split the source field back to lines and then deduplicate these lines.
3. Do a call to wrap_sources to nicely wrap things.

Both 2 and 3 could be done after adding all messages to the catalog — I think that would give us linear complexity here. Right now, we have some potential for O(n²) complexity here.

Putting a filename per line remove ambiguity about filenames that contain spaces. I'm not actually sure how such filenames ought to be stored in the sources field — feel free to experiment a bit with xgettext if you are curious!

[zachcmadsen]

Thanks for the outline @mgeisler! I pushed an update along those lines. What I have assumes that duplicate sources are consecutive. Are there cases where that doesn't hold?

[mgeisler]

What I have assumes that duplicate sources are consecutive. Are there cases where that doesn't hold?

I think it should hold right now — we walk over the contents of the book in the order given by Book::iter. It says that it's doing a depth-first traversal on the files of the book, so it ought to do the right thing!

[kdarkhan]

This change looks good to me.

Here is the diff of the generated messages.pot from comprehensive-rust repo.

https://gist.github.com/kdarkhan/9e270ae1e5842abd10f826dcaa5940aa

[kdarkhan]

Thanks for contribution @zachcmadsen! Feel free to pick up the followup task #171 if you want.