Skip to content

refactor: replace ARGS_ with ARG_, standardize requireParcelable usage#21366

Merged
criticalAY merged 2 commits into
ankidroid:mainfrom
ericli3690:ericli3690-naming-standardization
Jul 13, 2026
Merged

refactor: replace ARGS_ with ARG_, standardize requireParcelable usage#21366
criticalAY merged 2 commits into
ankidroid:mainfrom
ericli3690:ericli3690-naming-standardization

Conversation

@ericli3690

Copy link
Copy Markdown
Member

Note

Assisted-by: Composer 2.5 (for ARG_ renaming grunt work)

Purpose / Description

Per the naming conventions we've decided on, argument constant names should begin with ARG_, not ARGS_. This change standardizes instances of ARGS_ I found in the codebase. I initially tried applying the change to all argument constants in the entire repo, but as you can guess that spiralled out of control really fast. To keep the scope limited and safe, I've just looked for all instances of "ARGS_", plus any files I've touched, and fixed those.

Also, at #19896, David suggested using requireParcelable more. I find the utility functions in BundleUtils very useful and have applied them everywhere possible after looking for instances of requireNotNull in the codebase. This includes two review reminder instances.

Fixes

Approach

Renaming PR, followed by the standardization of requireParcelable, requireString, etc.
There were some requireNotNull instances that grabbed things like string arrays and long arrays from Bundles: no such utility function to handle these cases exists yet in BundleUtils so I just left those as is. Could be something to return to in the future, but this is very low priority.

I also believe it is better to use lazy to get stored arguments rather than defining a get() = to avoid re-computation of something that never changes.

Overall, after applying requireParcelable and lazy, I personally think a lot of argument usages are now cleaner.

How Has This Been Tested?

  • App builds
  • Unit tests pass
  • Card browser and note editor works: editing cards, moving columns, selecting columns, search, tags search, adding fields
  • Review reminders can be edited, deleted (includes confirmation dialog); notification permission requests work

Checklist

  • 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

Per ankidroid#19896, argument constants should begin with ARG_. Many files, including some I've created, use ARGS_. This commit fixes that.

Assisted-by: Composer 2.5
There are useful extensions in BundleUtils courtesy of David (thanks David!) for accessing stored values in argument Bundles. Using them throughout the codebase significantly cleans up several call sites. I also believe it is better to use `lazy` to get stored arguments rather than defining a `get() =` to avoid re-computation of something that never changes.
) : ViewModel() {
val args: ImageOcclusionArgs =
checkNotNull(savedStateHandle[IO_ARGS_KEY]) { "$IO_ARGS_KEY required" }
checkNotNull(savedStateHandle[ARG_IMAGE_OCCLUSION]) { "$ARG_IMAGE_OCCLUSION required" }

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.

This could have an extension to be cleaner

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hm sorry, not sure what you mean / are envisioning here. Do you want to create something similar to BundleUtil's helper functions for SavedStateHandles, too? Something like SavedStateHandleUtil.kt and "requireSavedStateKey"? That's a bit overkill / out of scope for this PR in my opinion, this is probably fine for now. Unless you meant something else?

@david-allison

Copy link
Copy Markdown
Member

Merge at your discretion, might warrant a second Reviewer as I'm unsure of the test coverage here

@david-allison david-allison added Needs Second Approval Has one approval, one more approval to merge and removed Needs Review labels Jul 12, 2026
@ericli3690

Copy link
Copy Markdown
Member Author

Let's wait for one more reviewer just in case! Thanks for the quick review :)

@criticalAY criticalAY left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@criticalAY
criticalAY added this pull request to the merge queue Jul 13, 2026
@criticalAY criticalAY added Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) and removed Needs Second Approval Has one approval, one more approval to merge labels Jul 13, 2026
Merged via the queue into ankidroid:main with commit 95ea64b Jul 13, 2026
20 checks passed
@github-actions github-actions Bot added this to the 2.25 release milestone Jul 13, 2026
@github-actions github-actions Bot removed the Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) label Jul 13, 2026
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.

3 participants