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

feat: allow deleting variables used in expressions #4860

Merged
merged 3 commits into from
Feb 13, 2025
Merged

Conversation

TrySound
Copy link
Member

Ref #4768

Before we disabled deleting variables which are used in expressions. Now we will ask confirmation and unset this variable. This way user will not be locked if variable is lost and we will show diagnostics (in the future) where all unset variables are.

Screenshot 2025-02-12 at 15 38 14

Ref #4768

Before we disabled deleting variables which are used in expressions.
Now we will ask confirmation and unset this variable. This way user will
not be locked if variable is lost and we will show diagnostics (in the
future) where all unset variables are.
@TrySound TrySound requested review from kof and johnsicili February 12, 2025 07:38
@kof
Copy link
Member

kof commented Feb 12, 2025

I think search should be able to find the variables that are used but are unset, please create an issue or add a todo to a related issue

@kof
Copy link
Member

kof commented Feb 12, 2025

Also is there a way to show any hint which instance or variable is using it right in that dialog?

@kof
Copy link
Member

kof commented Feb 12, 2025

At least we could show instance names, at best we would make instance names clickable to jump to that instance

Copy link
Contributor

@johnsicili johnsicili left a comment

Choose a reason for hiding this comment

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

  1. I would make the dialog title sentence case.
  2. I would include the variable name somewhere in the dialog too so the user is super sure of the variable they are deleting
Screenshot 2025-02-12 at 8 16 50 AM

@TrySound
Copy link
Member Author

image

@TrySound TrySound merged commit ae864ef into main Feb 13, 2025
17 checks passed
@TrySound TrySound deleted the delete-variable branch February 13, 2025 02:37
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