#315 fix: cursor error when go-blueprint create fails

[vinitparekh17]

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

Problem/Feature

This PR resolves issue #315

Description of Changes:

In the CreateMainFile function located in /cmd/program/program.go, the error handling has been updated to ensure the terminal is properly released. Specifically:

• The function now returns an err so that the spinner.ReleaseTerminal() call can be executed ( create.go line 273 ), ensuring the terminal is released even in the event of an error.

This change helps prevent terminal lock-ups by ensuring proper cleanup

Checklist

• I have self-reviewed the changes being requested

[Ujstor]

Your PR didn't solve the issue. Try testing it, for example, by using a path where go.mod doesn't exist in program.go on purpose:

	// Create go.mod
	err = utils.InitGoMod(p.ProjectName, gitHubActionPath)
	if err != nil {
		log.Printf("Could not initialize go.mod in new project %v\n", err)
		cobra.CheckErr(err)
		return err
	}

go build
go install

Create the project....blueprint errors out, and the terminal still doesn't display visible input.

[vinitparekh17]

Your PR didn't solve the issue. Try testing it, for example, by using a path where go.mod doesn't exist in program.go on purpose:

	// Create go.mod
	err = utils.InitGoMod(p.ProjectName, gitHubActionPath)
	if err != nil {
		log.Printf("Could not initialize go.mod in new project %v\n", err)
		cobra.CheckErr(err)
		return err
	}

go build go install

Create the project....blueprint errors out, and the terminal still doesn't display visible input.

Code snippet: err = utils.InitGoMod(p.ProjectName, gitHubActionPath)

Thank you for the code snippet. I understand your concern about the potential bug scenario. However, as per my understanding we do not offer any flag to ask user for project path so this case would only occur if someone intentionally modifies the code to match the provided snippet, which would deviate from our intended implementation. Should we still add additional safeguards for such edge cases, or do you think the current implementation sufficiently handles the expected use cases?

[vinitparekh17]

@Ujstor I would like to know your thoughts

[Ujstor]

My point was that returning the "raw" error will not solve the issue. I stretched my example just to prove the point. I don't know what went wrong in #315, but it can happen, and users will experience the issue. If you completely remove the spinner from the codebase, the bug disappears. That's how I know the spinner itself causes this issue.

In the end, we have duplication in the codebase, there is no need to return an error and use cobra.CheckErr. The codebase is the result of work from many contributors, and things like this sneak in over time. Maybe it's time for a refactor ;)

[vinitparekh17]

My point was that returning the "raw" error will not solve the issue. I stretched my example just to prove the point. I don't know what went wrong in #315, but it can happen, and users will experience the issue. If you completely remove the spinner from the codebase, the bug disappears. That's how I know the spinner itself causes this issue.

In the end, we have duplication in the codebase, there is no need to return an error and use cobra.CheckErr. The codebase is the result of work from many contributors, and things like this sneak in over time. Maybe it's time for a refactor ;)

Indeed

[Ujstor]

@vinitparekh17 Would you like to dig deeper into the problem and try to find a possible solution, or should I close the PR?

[vinitparekh17]

@vinitparekh17 Would you like to dig deeper into the problem and try to find a possible solution, or should I close the PR?

I would like to resolve this issue, might need some time and suggestions

[vinitparekh17]

@Ujstor I've tested it with your snippet, and it's now working without breaking. I've just updated the error handling in create.go to use cobra.CheckErr instead of handling it in the CreateMainFile function.

[vinitparekh17]

@Ujstor I've tested it with your snippet, and it's now working without breaking. I've just updated the error handling in create.go to use cobra.CheckErr instead of handling it in the CreateMainFile function.

simple explaination

cobra.CheckErr was printing an error message and exiting with code 1, which caused this line to never be executed, leading to the bug we were facing.

[Ujstor]

Unfortunately, this is not a valid solution. The cobra library is one of the core components, and it is used for a reason.

[vinitparekh17]

Unfortunately, this is not a valid solution. The cobra library is one of the core components, and it is used for a reason.

Indeed and I just made sure that we use cobra.CheckErr once at create.go rather than multiple times in program.go so we can execute these lines spinner.ReleaseTerminal() before getting terminated

err = project.CreateMainFile()
		if err != nil {
			if releaseErr := spinner.ReleaseTerminal(); releaseErr != nil {
				log.Printf("Problem releasing terminal: %v", releaseErr)
			}
			log.Printf("Problem creating files for project. %v", err)
			cobra.CheckErr(textinput.CreateErrorInputModel(err).Err())
		}
		

I manually tested this with your snippet as well as by my own scenarios and it shows error as it was, before releasing cursor.

[Ujstor]

Thanks for the feedback. I need to review this in detail, and I hope to manage it by the end of the week.

[Ujstor]

I checked and tested your implementation, and you are right, the error was not handled correctly in program.go. Very, very nice bug hunt!

LGTM!!