Fetch/Init/Clone command now can handle Ctrl+C/Break key #517

Open
wants to merge 6 commits into
from

Conversation

Projects
None yet
2 participants
@KindDragon
Contributor

KindDragon commented Dec 25, 2013

No description provided.

@spraints

This comment has been minimized.

Show comment Hide comment
@spraints

spraints Jan 15, 2014

Member

Cool.

What's the main change with this? What things happen on Ctrl-C now that didn't happen before?

Is there a way to have the token just in the places where it's used? Maybe by adding the CancellationToken to the StructureMap container as a singleton? Or, maybe flip the logic around. Instead of having to check in with the CancellationToken, create a way to defer the interrupt when there's something that shouldn't be interrupted.

Member

spraints commented Jan 15, 2014

Cool.

What's the main change with this? What things happen on Ctrl-C now that didn't happen before?

Is there a way to have the token just in the places where it's used? Maybe by adding the CancellationToken to the StructureMap container as a singleton? Or, maybe flip the logic around. Instead of having to check in with the CancellationToken, create a way to defer the interrupt when there's something that shouldn't be interrupted.

@KindDragon

This comment has been minimized.

Show comment Hide comment
@KindDragon

KindDragon Jan 16, 2014

Contributor

What's the main change with this? What things happen on Ctrl-C now that didn't happen before?

Old code doesn't handle Ctrl-C at all and .Net just terminate app when user press Ctrl+C. New code set Cancel state in CancelationToken and git-tfs command should check this token to terminate app.

Contributor

KindDragon commented Jan 16, 2014

What's the main change with this? What things happen on Ctrl-C now that didn't happen before?

Old code doesn't handle Ctrl-C at all and .Net just terminate app when user press Ctrl+C. New code set Cancel state in CancelationToken and git-tfs command should check this token to terminate app.

@KindDragon

This comment has been minimized.

Show comment Hide comment
@KindDragon

KindDragon Jan 16, 2014

Contributor

Is there a way to have the token just in the places where it's used? Maybe by adding the CancellationToken to the StructureMap container as a singleton?

I'll try to do it, but I had not previously worked with StructureMap

Instead of having to check in with the CancellationToken, create a way to defer the interrupt when there's something that shouldn't be interrupted.

The user did not see the response to pressing Ctrl + C. I added processing button when pressing Ctrl + C program for a long time did not response.

Contributor

KindDragon commented Jan 16, 2014

Is there a way to have the token just in the places where it's used? Maybe by adding the CancellationToken to the StructureMap container as a singleton?

I'll try to do it, but I had not previously worked with StructureMap

Instead of having to check in with the CancellationToken, create a way to defer the interrupt when there's something that shouldn't be interrupted.

The user did not see the response to pressing Ctrl + C. I added processing button when pressing Ctrl + C program for a long time did not response.

@spraints

This comment has been minimized.

Show comment Hide comment
@spraints

spraints Jan 17, 2014

Member

What's the main change with this? What things happen on Ctrl-C now that didn't happen before?

Old code doesn't handle Ctrl-C at all and .Net just terminate app when user press Ctrl+C. New code set Cancel state in CancelationToken and git-tfs command should check this token to terminate app.

I gathered that. But what does that lead to? Are there things that don't get cleaned up? I really like the idea of handling ctrl-C gracefully. I just want to balance this change (currently it adds code to deal with a single concern to 39 files) against its functional value (???).

What I'm trying to do is this:

  • keep the amount of time between pressing ctrl-C and exit as short as possible. I don't like pressing ctrl-C and having it look like the program just ignores it. I guess another option would be to do something like what rspec does: "Shutting down gracefully. Press Ctrl-C again to exit immediately." The second ctrl-C press would not set e.Cancel.
  • avoid having to remember to add code to handle a ctrl-C. As a git-tfs coder, I want ctrl-C to act like an exception. I can add code to handle it at the point in the stack where it makes sense to handle it, rather than checking exit codes all over.

Is there a way to translate ctrl-C into an exception in just one place, rather than sprinkled throughout the project? e.g. would this work as the only change?

                Console.CancelKeyPress += delegate(object sender,
                                        ConsoleCancelEventArgs e)
                {
                    Trace.WriteLine("The operation has been interrupted.");
                    if (e.SpecialKey == ConsoleSpecialKey.ControlC)
                        e.Cancel = true; // tell the CLR to keep running
                    throw new Exception("Interrupt");
                };
Member

spraints commented Jan 17, 2014

What's the main change with this? What things happen on Ctrl-C now that didn't happen before?

Old code doesn't handle Ctrl-C at all and .Net just terminate app when user press Ctrl+C. New code set Cancel state in CancelationToken and git-tfs command should check this token to terminate app.

I gathered that. But what does that lead to? Are there things that don't get cleaned up? I really like the idea of handling ctrl-C gracefully. I just want to balance this change (currently it adds code to deal with a single concern to 39 files) against its functional value (???).

What I'm trying to do is this:

  • keep the amount of time between pressing ctrl-C and exit as short as possible. I don't like pressing ctrl-C and having it look like the program just ignores it. I guess another option would be to do something like what rspec does: "Shutting down gracefully. Press Ctrl-C again to exit immediately." The second ctrl-C press would not set e.Cancel.
  • avoid having to remember to add code to handle a ctrl-C. As a git-tfs coder, I want ctrl-C to act like an exception. I can add code to handle it at the point in the stack where it makes sense to handle it, rather than checking exit codes all over.

Is there a way to translate ctrl-C into an exception in just one place, rather than sprinkled throughout the project? e.g. would this work as the only change?

                Console.CancelKeyPress += delegate(object sender,
                                        ConsoleCancelEventArgs e)
                {
                    Trace.WriteLine("The operation has been interrupted.");
                    if (e.SpecialKey == ConsoleSpecialKey.ControlC)
                        e.Cancel = true; // tell the CLR to keep running
                    throw new Exception("Interrupt");
                };
@KindDragon

This comment has been minimized.

Show comment Hide comment
@KindDragon

KindDragon Jan 18, 2014

Contributor

Ctrl+C - Shutting down gracefully
Ctrl+Break - Exit immediately

Is there a way to translate ctrl-C into an exception in just one place, rather than sprinkled throughout the project? e.g. would this work as the only change?

No, your code will be called from separate thread and terminate application immediately.

Contributor

KindDragon commented Jan 18, 2014

Ctrl+C - Shutting down gracefully
Ctrl+Break - Exit immediately

Is there a way to translate ctrl-C into an exception in just one place, rather than sprinkled throughout the project? e.g. would this work as the only change?

No, your code will be called from separate thread and terminate application immediately.

@spraints

This comment has been minimized.

Show comment Hide comment
@spraints

spraints Jan 20, 2014

Member

Ctrl+C - Shutting down gracefully

🆒 But what does that mean?

Member

spraints commented Jan 20, 2014

Ctrl+C - Shutting down gracefully

🆒 But what does that mean?

@KindDragon

This comment has been minimized.

Show comment Hide comment
@KindDragon

KindDragon Jan 20, 2014

Contributor

We should handle CancellationToken in each command. Often call token.ThrowIfCancellationRequested()

Contributor

KindDragon commented Jan 20, 2014

We should handle CancellationToken in each command. Often call token.ThrowIfCancellationRequested()

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