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

Do not crash when nullable actions is null #236

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

babbage
Copy link

@babbage babbage commented Jan 22, 2024

Actions was made nullable in d93e351 but that same commit introduced a non-null assertion operator here which means it is valid to create a KeyboardActionsConfig with a null actions but this will crash KeyboardActions on init.

This PR ensures that a check is made if there are actions on the newConfig and only attempt to iterate over them when they exist. By assigning newConfig.actions to a local variable it is implicitly unwrapped once the null check has occurred.

The PR also removes the flutter_export_environment.sh file from the repo and adds it to the .gitignore file, as this file is not intended to be committed to source control.

Actions was made nullable in d93e351 but that same commit introduced a non-null assertion operator here which means it is valid to create a `KeyboardActionsConfig` with a null `actions` but this will crash `KeyboardActions` on init.

This commit ensures that we check if there are actions on the newConfig and only attempt to iterate over them when they exist. By assigning newConfig.actions to a local variable it is implicitly unwrapped once the null check has occurred.
This is a generated file that is not intended to be checked into version control. It is deleted here to remove it from the repo and added to `.gitignore` but will be re-generated locally (and ignored from source control) as each developer builds the project.
@babbage babbage changed the title Do no crash when nullable actions is null Do not crash when nullable actions is null Jan 22, 2024
Copy link

@StoneNan StoneNan left a comment

Choose a reason for hiding this comment

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

LGTM

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