Skip to content

refactor: extract AnkiDroidApp.instance to :common:android#21164

Open
criticalAY wants to merge 1 commit into
ankidroid:mainfrom
criticalAY:widget/appcontext
Open

refactor: extract AnkiDroidApp.instance to :common:android#21164
criticalAY wants to merge 1 commit into
ankidroid:mainfrom
criticalAY:widget/appcontext

Conversation

@criticalAY
Copy link
Copy Markdown
Contributor

Purpose / Description

Extract application context to :common:android, a step towards clean code and one more step closer to widget extraction

Fixes

Approach

See commit

How Has This Been Tested?

Local build and ran on Emualtor works smooth

Learning (optional, can help others)

NA

Checklist

Please, go through these checks before submitting the PR.

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

fun toBundle(): Bundle =
bundleOf(
INTENT_MODEL_FILENAME to saveTempNoteType(AnkiDroidApp.instance.applicationContext, notetype),
INTENT_MODEL_FILENAME to saveTempNoteType(AnkiDroidApplication.instance.applicationContext, notetype),
Copy link
Copy Markdown
Member

@david-allison david-allison May 28, 2026

Choose a reason for hiding this comment

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

TODO: cleanup in another commit - applicationContext is not necessary, this is an Applicaton

// SPDX-License-Identifier: GPL-3.0-or-later
// SPDX-FileCopyrightText: 2026 Ashish Yadav <mailtoashish693@gmail.com>

package com.ichi2.anki.common.application
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd use common.android unless you feel a lot will be in this class


val isInitialized: Boolean
get() = ::instance.isInitialized
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Disagree with me here:

I'd prefer this if it were a bare appContext variable.

- AnkiDroidApplication.instance.contentResolver
+ appContext.contentResolver
  1. rename current usages of appContext

  2. something like this

/** */
lateinit var appContext: Context
    private set

object AnkiDroidApplication {
    /** Sets the [appContext]. Called once from `AnkiDroidApp.onCreate()`. */
    fun setApplicationInstance(app: Application) {
        appContext = app
    }
}

@david-allison david-allison added the Needs Author Reply Waiting for a reply from the original author label May 28, 2026
@criticalAY criticalAY force-pushed the widget/appcontext branch from 878e3e4 to bdfbabc Compare May 28, 2026 20:53
@criticalAY criticalAY removed the Needs Author Reply Waiting for a reply from the original author label May 28, 2026
Copy link
Copy Markdown
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

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

Clean out + rename the existing non-test appContext variables before the commit declaring + using the global accessor.

AnkiDroidUsageAnalytics.appContext for example has special consideration, as instance may not have been set

…idApplication

Adds a top-level `appContext: Context` accessor in :common:android, set
during AnkiDroidApp.onCreate() via AnkiDroidApplication.setApplicationInstance.
Shared and feature-module code can now reach the Application context without
depending on :AnkiDroid's AnkiDroidApp subclass.
@criticalAY criticalAY force-pushed the widget/appcontext branch from bdfbabc to ee10d2c Compare May 28, 2026 23:21
Copy link
Copy Markdown
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

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

One issue, then LGTM

The TODO is optional

Leaving this for second approval as it's a common pattern we want to get right (naming etc...) first time

var templateChanges = ArrayList<TemplateChange>()
private set

// TODO: cleanup applicationContext is not necessary, this is an Application
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

fixed

} else {
Timber.w("AnkiDroid instance not set")
null
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  1. AnkiDroidApplication.isInitialized I believe
  2. I'd prefer not exposing isInitalized and use AnkiDroidApplication.instanceOrNull (or similarly named variables)
    • as a TODO/separate commit
  val instanceOrNull: Context?
      get() = if (::appContext.isInitialized) appContext else null

@david-allison david-allison added Needs Author Reply Waiting for a reply from the original author Needs Second Approval Has one approval, one more approval to merge and removed Needs Review labels May 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Author Reply Waiting for a reply from the original author Needs Second Approval Has one approval, one more approval to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants