Anki-Android icon indicating copy to clipboard operation
Anki-Android copied to clipboard

Audit code for consistency in our Intent back stack management

Open mikehardy opened this issue 1 year ago • 11 comments

          There is a lack of consistency in our Intent back stack management

https://github.com/search?q=repo%3Aankidroid%2FAnki-Android+Intent.FLAG_ACTIVITY_CLEAR&type=code

...shows we use CLEAR_TASK sometimes, an or if flags other times, and CLEAR_TOP other times

All intent creation https://github.com/search?q=repo%3Aankidroid%2FAnki-Android+Intent%28context%2C+&type=code

look at:

  • any case where we create an Intent to start an activity
  • ...to verify that we are actually using the correct back stack management flags

Originally posted by @mikehardy in https://github.com/ankidroid/Anki-Android/pull/17036#pullrequestreview-2306753955

mikehardy avatar Sep 16 '24 13:09 mikehardy

The description of this issue is the problem statement The implementation may be done differently, but it seems - based on the usage being inconsistent in lots of places and this being an area not many people seem to know about + appears hard to get correct - we could do with a couple utility functions

One would be a "startIntent" utility where you pass the class you want to start and some extras, and it would do the CLEAR_TOP etc

Another would be the case that appears to also pretty regular with the NEW_TASK and some other flag or'd together

Then scour the code for Intent creation and funnel it all through the utility methods and we would know that they were all behaving the same (and hopefully correctly...)

mikehardy avatar Sep 16 '24 17:09 mikehardy

Thanks for self assigning @xenonnn4w. I thought you were too busy with back to school currently to contribute.

Arthur-Milchior avatar Sep 17 '24 10:09 Arthur-Milchior

I think it may be related to #17083, where the stack don't always work as I expect, and this is frustrating.

Arthur-Milchior avatar Sep 17 '24 11:09 Arthur-Milchior

Apologies, but I won't be able to work on this for at least the next two weeks as I'm busy with my electronics projects and subjects.

xenonnn4w avatar Sep 25 '24 10:09 xenonnn4w

Never a problem! This is a "coding standards" / "consistency" exercise. There are probably some minor issues lurking around with our current lack of doing it the same way everywhere, but users aren't screaming about it...

Good luck on your projects

mikehardy avatar Sep 25 '24 16:09 mikehardy

The description of this issue is the problem statement The implementation may be done differently, but it seems - based on the usage being inconsistent in lots of places and this being an area not many people seem to know about + appears hard to get correct - we could do with a couple utility functions

One would be a "startIntent" utility where you pass the class you want to start and some extras, and it would do the CLEAR_TOP etc

Another would be the case that appears to also pretty regular with the NEW_TASK and some other flag or'd together

Then scour the code for Intent creation and funnel it all through the utility methods and we would know that they were all behaving the same (and hopefully correctly...)

@mikehardy so the 2nd utility function should have a flag argument that takes other flag as input from the caller? and what about the places where CLEAR_TASK is used , and where CLEAR_TOP is or'd with SINGLE_TOP? should i leave them as it is?

Raghav1783 avatar Oct 01 '24 17:10 Raghav1783

Whatever seems the cleanest, if I spend too much time thinking about it I should just code it 😆

If I recall correctly the second case had same flags each time, so no need to parameterize

mikehardy avatar Oct 01 '24 18:10 mikehardy

  • Related: #10970

david-allison avatar Oct 02 '24 14:10 david-allison

on it

MinusMallard avatar Oct 07 '24 04:10 MinusMallard

@david-allison here is how I will do this, I am just going to create "IntentUtility" class with static functions and then replace each intent creation with function call

MinusMallard avatar Oct 07 '24 12:10 MinusMallard

I don't have a good mental model of how that will look, but give it a go

If it's quick, do it all

If it's not, provide a small patch fixing a few calls so we can see if it looks like a good approach

Whatever documentation/research you have done on the issue will definitely stand the test of time

david-allison avatar Oct 07 '24 17:10 david-allison

Is this still being worked on or can i give it a try?

BoredVoidEater avatar Nov 28 '24 13:11 BoredVoidEater

@MinusMallard are you still working on this?

david-allison avatar Nov 29 '24 04:11 david-allison

@BoredVoidEater No reply, yours if you want it.

david-allison avatar Nov 30 '24 05:11 david-allison

No updates from a month. So, can i work on this?

priyanshu-lgtm avatar Dec 25 '24 11:12 priyanshu-lgtm

Sure

david-allison avatar Dec 25 '24 16:12 david-allison

Working on it.

Akshit517 avatar Feb 07 '25 01:02 Akshit517

Hello 👋, this issue has been opened for more than 3 months with no activity on it. If the issue is still here, please keep in mind that we need community support and help to fix it! Just comment something like still searching for solutions and if you found one, please open a pull request! You have 7 days until this gets closed automatically

github-actions[bot] avatar May 08 '25 09:05 github-actions[bot]