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

Update fromLegacyString method #4604

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Update fromLegacyString method #4604

wants to merge 2 commits into from

Conversation

cooltey
Copy link
Collaborator

@cooltey cooltey commented Apr 17, 2024

  • Update the Namespace.fromLegacyString() and try to find the closest namespace from the entries.
  • Update PageSummary.getPageTitle() for a proper namespace
  • Use PageSumary.getPageTitle() for LinkPreviewViewModel.kt

Pending discussion:

  • Performance issue when searching namespace in entries?
  • Should we show the LinkPreviewDialog for pages in all namespaces?

@cooltey cooltey added the Experimental Experiments and prototypes label Apr 17, 2024
@@ -190,7 +193,7 @@ enum class Namespace(private val code: Int) : EnumCode {
return if (UserTalkAliasData.valueFor(wiki.languageCode).equals(name, true) ||
UserTalkAliasData.valueFor(AppLanguageLookUpTable.FALLBACK_LANGUAGE_CODE).equals(name, true)) {
USER_TALK
} else MAIN
} else entries.firstOrNull { it.name.equals(name, true) } ?: MAIN
Copy link
Member

Choose a reason for hiding this comment

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

This will only work for English. All namespaces have different localized names on each language wiki.

I think a better long-term solution will be to get rid of fromLegacyString() entirely, and have individual functions for isTalk(), isUser(), etc. and eliminate all the places where we attempt to check for namespace() == MAIN

Copy link
Member

Choose a reason for hiding this comment

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

...and then we could also remove >100 of the actual Namespace enum definitions that we don't actually use anywhere, too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Experimental Experiments and prototypes
Development

Successfully merging this pull request may close these issues.

2 participants