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

Removed comma separator #5261

Merged
merged 8 commits into from
Jul 20, 2023
Merged

Conversation

paco-arana
Copy link
Contributor

Description

Fixes #5260

area: fr.free.nrw.commons.description.DescriptionEditActivity#updateDescription

The comma separator between descriptions in different languages has been removed from fr.free.nrw.commons.description.DescriptionEditActivity#updateDescription line 154. This also made line 157 redundant as its function was deleting the very last comma after all the descriptions were formatted.

Tests performed

Tested beta build on Xiaomi Redmi 9 using Android 11.

Copy link
Member

@nicolas-raoul nicolas-raoul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I uploaded a file using your branch and still see a comma, that's strange: https://commons.wikimedia.org/wiki/File:Binh_Tay_market_2.jpg

Maybe DescriptionEditActivity is only used when modifying an existing file, rather than when uploading a new file?

Thanks! :-)

…}, " with "}}" to remove the comma between descriptions.
@@ -112,5 +112,6 @@ data class Contribution constructor(
fun formatDescriptions(descriptions: List<UploadMediaDetail>) =
descriptions.filter { it.descriptionText.isNotEmpty() }
.joinToString { "{{${it.languageCode}|1=${it.descriptionText}}}" }
.replace("}}, ", "}}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would not it be better to not add the comma in the first place?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that first but I was having a hard time determining where the comma was actually originating from, I'll dig a little more until I can find it.

Thanks for the quick response, I apologize if I'm making you work more than needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I think this one should do it, I should've checked the default separator for the .joinToString first before trying to look for the comma everywhere else.

Copy link
Member

@nicolas-raoul nicolas-raoul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I will test your pull request as soon as possible. 🙂

Copy link
Member

@nicolas-raoul nicolas-raoul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uploading now works great! :-)
Unfortunately editing adds the commas:
https://commons.wikimedia.org/w/index.php?title=File%3AKaba_amphibious_bus_in_Odaiba.jpg&diff=784750015&oldid=784749122
Would you mind checking?

@paco-arana
Copy link
Contributor Author

paco-arana commented Jul 19, 2023

I seem to have caused that myself when I modified DescriptionEditActivity#updateDescription, I added the deleted line back in and now it's working as it should:

https://commons.wikimedia.org/w/index.php?title=File:Catedral_de_Aguascalientes.jpg&diff=prev&oldid=784872825

That line is using .replace() though, is that fine for this case?

Copy link
Member

@nicolas-raoul nicolas-raoul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now working great for both upload and modification.
Example: https://commons.wikimedia.org/wiki/File:Central_Building,_Sakura_Tower.jpg
Thanks a lot!

@nicolas-raoul nicolas-raoul merged commit 3f529e6 into commons-app:master Jul 20, 2023
@paco-arana paco-arana deleted the removedcomma branch July 20, 2023 20:22
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.

[Bug]: Descriptions in multiple languages should not be separated by comma
2 participants