Skip to content

[Fix #438] Disallow empty project names #472

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

Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions datashuttle/tui/configs.py
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,16 @@ def widget_configs_match_saved_configs(self):
cfg_kwargs = self.get_datashuttle_inputs_from_widgets()

project_name = self.interface.project.cfg.project_name
# Add validation before proceeding
if not project_name.strip():
self.notify(
Copy link
Member

Choose a reason for hiding this comment

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

Nice! This is a cool textual feature, I think we could move towards using it one day. Currently we use modal dialog boxes to display errors, the class is here and called for example here. It achieves the same idea but is just more aligned with how the project currently displays errors for consistency. However, see my comment below I think we can get this for free with a change in a slightly different place in the codebase.

"Project name cannot be empty. Please enter a valid name.",
severity="error",
)
print(
Copy link
Member

Choose a reason for hiding this comment

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

It is okay to remove these print statements now, these are useful in development but if the error is already notified through the TUI these can be removed

"Project creation blocked due to empty name!"
) # Debugging print
return

for key, value in cfg_kwargs.items():
saved_val = self.interface.get_configs()[key]
Expand All @@ -446,6 +456,17 @@ def setup_configs_for_a_new_project(self) -> None:
with the new project.
"""
project_name = self.query_one("#configs_name_input").value
# Add validation before proceeding
if not project_name.strip():
Copy link
Member

Choose a reason for hiding this comment

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

Just as a general tip, if you have a piece of code you are using in one place, you can create a function and place the code there, then call it from multiple places. The benefit of this is that if you need to change the code in future, you only have to do it in one place.

self.notify(
"Project name cannot be empty. Please enter a valid name.",
severity="error",
)
print(
Copy link
Contributor

Choose a reason for hiding this comment

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

I think instead of using a print statement, it would be more helpful to display this message in a dialog box. That way, users are immediately informed about the issue without needing to check logs.

Copy link
Author

@chandrikarj chandrikarj Mar 3, 2025

Choose a reason for hiding this comment

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

Hi @parharti, well your suggestion makes sense, and, the users don't need to check logs and can already see the statement in a dialog box. I had added the print statement to debug the same. Should you wish to, you can see it in the video linked under demonstration of the changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

got your point!!

"Project creation blocked due to empty name!"
) # Debugging print
return

cfg_kwargs = self.get_datashuttle_inputs_from_widgets()

interface = Interface()
Expand Down