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

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

Open
mikehardy opened this issue Sep 16, 2024 · 11 comments
Open

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

mikehardy opened this issue Sep 16, 2024 · 11 comments

Comments

@mikehardy
Copy link
Member

          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 #17036 (review)

@mikehardy
Copy link
Member Author

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
Copy link
Member

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

@Arthur-Milchior
Copy link
Member

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

@xenonnn4w
Copy link
Contributor

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 xenonnn4w removed their assignment Sep 25, 2024
@mikehardy
Copy link
Member Author

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
Copy link

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
Copy link
Member Author

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
Copy link
Member

@MinusMallard
Copy link
Contributor

on it

@MinusMallard
Copy link
Contributor

@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
Copy link
Member

david-allison commented Oct 7, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants