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

Fixes duplicates entries in XML resources #995

Merged
merged 6 commits into from
Oct 25, 2020
Merged

Fixes duplicates entries in XML resources #995

merged 6 commits into from
Oct 25, 2020

Conversation

bagipro
Copy link
Collaborator

@bagipro bagipro commented Oct 25, 2020

❗ Please review the guidelines for contributing

Description

#994

@bagipro
Copy link
Collaborator Author

bagipro commented Oct 25, 2020

image

@skylot
Copy link
Owner

skylot commented Oct 25, 2020

@sergey-wowwow, unfortunately, you can't use binary search on that list because it must be sorted before search with the same comparator, and that not true here. I'm not yet sure how to better fix this so I just replace search with plain loop. Please check that it is still working as intended.

@jpstotz jpstotz changed the title Fixes dublicates entries in XML resources Fixes duplicates entries in XML resources Oct 25, 2020
@skylot
Copy link
Owner

skylot commented Oct 25, 2020

I just notice that we need to add config variable to name comparator. Othewise it renames all entries in other configs even they are not obfuscated. Also this make entries with same id have different names in different configs.
As soon this issue become more complex here some constraint we must met:

  • entry names in one config and type must be unique
  • entry name for same id must be preserved across different configs
  • notice that different configs can have different entry sets

I tried to fix this PR according to these constatraints, but not sure if it cover all possible cases. Please check and review :)

@bagipro
Copy link
Collaborator Author

bagipro commented Oct 25, 2020

@sergey-wowwow, unfortunately, you can't use binary search on that list because it must be sorted before search with the same comparator, and that not true here. I'm not yet sure how to better fix this so I just replace search with plain loop. Please check that it is still working as intended.

Late night coding issues :D

@skylot
Copy link
Owner

skylot commented Oct 25, 2020

@sergey-wowwow great, thanks!
Is this PR complete? Can I merge it?
Also, which issues can be closed after merge? All these: #994 #996 #952 right?

@bagipro
Copy link
Collaborator Author

bagipro commented Oct 25, 2020

@skylot
Yeah, I think it's complete and ready to be merged. I closed by issues, but not sure if #952 is also done, waiting for confirmation from @alissonlauffer

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.

2 participants