ankidroid / Anki-Android

AnkiDroid: Anki flashcards on Android. Your secret trick to achieve superhuman information retention.
GNU General Public License v3.0
8.66k stars 2.24k forks source link

Audit code for consistency in our Intent back stack management #17073

Open mikehardy opened 1 month ago

mikehardy commented 1 month ago
          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:

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

mikehardy commented 1 month ago

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...)

Arthur-Milchior commented 1 month ago

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

Arthur-Milchior commented 1 month ago

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

xenonnn4w commented 1 month ago

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.

mikehardy commented 1 month ago

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

Raghav1783 commented 1 month ago

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?

mikehardy commented 1 month ago

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

david-allison commented 1 month ago
MinusMallard commented 1 month ago

on it

MinusMallard commented 1 month ago

@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

david-allison commented 1 month ago

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