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

tag: Display error message when creating tag with invalid name (fix issue: #286) #296

Merged
merged 3 commits into from Jun 14, 2014

Conversation

Projects
None yet
2 participants
@mmargoliono
Contributor

mmargoliono commented Jun 9, 2014

When an invalid name is supplied during tag creation, there was no error
displayed to the user. It would always say "Created a new tag named [tag
name]" even though no tag is actually created.

This change will display error message instead of tag creation message
when git tag command fails.

Reported-by: V字龍(Vdragon) pika1021@gmail.com
Signed-off-by: Minarto Margoliono lie.r.min.g@gmail.com

tag: Display error message when creating tag with invalid name
When an invalid name is supplied during tag creation, there was no error
displayed to the user. It would always say "Created a new tag named [tag
name]" even though no tag is actually created.

This change will display error message instead of tag creation message
when git tag command fails.

Reported-by: V字龍(Vdragon) <pika1021@gmail.com>
Signed-off-by: Minarto Margoliono <lie.r.min.g@gmail.com>
@davvid

This comment has been minimized.

Show comment
Hide comment
@davvid

davvid Jun 10, 2014

Thanks, that looks pretty close. I think this should use Interaction.critical() instead, though, so that it's displayed as an error. Would you mind adjusting this so that we can use critical() instead?

Thanks, that looks pretty close. I think this should use Interaction.critical() instead, though, so that it's displayed as an error. Would you mind adjusting this so that we can use critical() instead?

This comment has been minimized.

Show comment
Hide comment
@mmargoliono

mmargoliono Jun 10, 2014

Owner

Sure, I will do it later today.

Owner

mmargoliono replied Jun 10, 2014

Sure, I will do it later today.

This comment has been minimized.

Show comment
Hide comment
@davvid

davvid Jun 10, 2014

Another thought... having the command return (status, out, err) should let us move the code that handles making the message strings down into the widget code.

That way we don't have the command handling the error message and the widget handling the success message. It's best for the UI code to handle all the message related stuff, and the command can be simple and just return enough state so that the UI code can handle it. What do you think?

Another thought... having the command return (status, out, err) should let us move the code that handles making the message strings down into the widget code.

That way we don't have the command handling the error message and the widget handling the success message. It's best for the UI code to handle all the message related stuff, and the command can be simple and just return enough state so that the UI code can handle it. What do you think?

This comment has been minimized.

Show comment
Hide comment
@mmargoliono

mmargoliono Jun 10, 2014

Owner

That is a good idea. I don't like the message in different places as well.

Owner

mmargoliono replied Jun 10, 2014

That is a good idea. I don't like the message in different places as well.

@davvid davvid referenced this pull request Jun 10, 2014

Closed

lacks git tag error dialog #286

mmargoliono added some commits Jun 10, 2014

Tag: Fix error message display on tag creation
The error message was displayed using information dialog. As a result,
it is displaying git-cola icon instead of error icon.

This change use the proper error dialog instead of information dialog.

Reported-by: V字龍(Vdragon) <pika1021@gmail.com>
Signed-off-by: Minarto Margoliono <lie.r.min.g@gmail.com>
Tag: Move error handling code to widget
The error message was handled in cmds.py while the success code was
handled in createtag.py. This will make it harder to maintain later.

This change move the error handling code to the same location as success
handling code.

Reported-by: V字龍(Vdragon) <pika1021@gmail.com>
Signed-off-by: Minarto Margoliono <lie.r.min.g@gmail.com>

davvid added a commit that referenced this pull request Jun 14, 2014

Merge pull request #296 from mmargoliono/286
tag: Display error message when creating tag with invalid name

Closes #286

Signed-off-by: David Aguilar <davvid@gmail.com>

@davvid davvid merged commit 332fcfa into git-cola:master Jun 14, 2014

@mmargoliono mmargoliono deleted the mmargoliono:286 branch Jun 14, 2014

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