Skip to content

Deal with parameters without a name #3046

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

Merged
merged 2 commits into from
Jan 24, 2024
Merged

Deal with parameters without a name #3046

merged 2 commits into from
Jan 24, 2024

Conversation

amolenaar
Copy link
Member

@amolenaar amolenaar commented Jan 23, 2024

PR Type

What kind of change does this PR introduce?

  • Bug fix
  • Feature
  • Chore (refactoring, formatting, local variables, other cleanup)
  • Documentation content changes

What is the current behavior?

If a parameter/attribute/operation/enumeration name is empty, the placeholder text is shown.

Issue Number: Reported on Matrix

What is the new behavior?

Show an empty cell.

image

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

This way it's clear the name is empty and the TextField doesn't
default to the placeholder text.
@github-actions github-actions bot added the python Pull requests that update Python code label Jan 23, 2024
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

PR Type: Bug fix

PR Summary: The pull request aims to address the issue where empty parameter, attribute, operation, and enumeration names were being replaced with placeholder text. The proposed change is to show an empty cell instead.

Decision: Comment

📝 Type: 'Bug fix' - not supported yet.
  • Sourcery currently only approves 'Typo fix' PRs.
✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.
No details provided.
✅ Small diff: the diff is small enough to approve with confidence.
No details provided.

General suggestions:

  • Ensure that returning a single space instead of an empty string is consistent with the application's handling of null or empty values. This change could have broader implications for UI behavior and data processing.
  • Consider the implications of localization for the hardcoded 'New Diagram' string. It may be beneficial to use a localized string to accommodate different languages.
  • Review the changes for potential side effects in the UI, particularly where the single space string is used as a default value. It's important to maintain a consistent user experience.

Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨

Share Sourcery

Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

Copy link
Member

@danyeaw danyeaw left a comment

Choose a reason for hiding this comment

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

Nice fix! Thanks @amolenaar!

@danyeaw danyeaw added fix A fix for a bug and removed python Pull requests that update Python code labels Jan 24, 2024
@danyeaw danyeaw merged commit f7320e8 into main Jan 24, 2024
@danyeaw danyeaw deleted the empty-parameters branch January 24, 2024 02:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix A fix for a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants