GemTalk / Jadeite

IDE for GemStone Smalltalk application development using Rowan code management
MIT License
6 stars 2 forks source link

Commit when no changes results in second dialog saying it did commit [was: walkback] #317

Open LisaAlmarode opened 6 years ago

LisaAlmarode commented 6 years ago

If you do a commit and git finds no changes, it puts up a walkback. It's not very friendly. Some of us who like to commit "just in case".

a UserDefinedError occurred (error 2318), reason:halt, performOnServer: 'set -e; cd $GEMSTONE/TomatoProject4//Tomato4; git --git-dir $GEMSTONE/TomatoProject4//Tomato4/.git --work-tree $GEMSTONE/TomatoProject4//Tomato4 commit --file=/tmp/commitMessage' stdout: 'On branch master nothing to commit, working directory clean ' failed with status: 256 errno: 0 errStr: nil

UserDefinedError (AbstractException) >> _signalWith: @5 line 25 UserDefinedError (AbstractException) >> signal @2 line 47 RwGitTool (Object) >> error: @6 line 7 [] in ExecBlock1 (RwGitTool) >> performOnServer:logging: @24 line 21 RwGitTool >> performOnServer:status: @4 line 4 RwGitTool >> performOnServer:logging: @2 line 4 RwGitTool >> performGitCommand:in:worktree:with:logging: @13 line 9 RwGitTool >> performGitCommand:in:with: @2 line 4 RwGitTool >> gitcommitIn:with: @2 line 3 RwPrjCommitTool (RwAbstractTool) >> doGitCommit: @11 line 8 RwGitRepositorySpecification >> commitForTool:message: @2 line 2 RwSimpleProjectSpecification (RwProjectSpecification) >> commitForTool:message: @3 line 2 RwPrjCommitTool >> commitProjectNamed:message: @18 line 16 RowanProjectService >> commitWithMessage: @7 line 5 GsNMethod class >> _gsReturnToC @1 line 1

dalehenrich commented 6 years ago

This will be something to consider for pyrite ... the error is actually coming from git (non-zero exit status). so it will take some work to figure which failures are going to be acceptable ... I assume that there is a git command that will answer whether or not a commit would succeed, so it would be something that would be added to the RowanProjectService >> commitWithMessage: so that the GUI can give appropriate feedback about an empty commit to the developer ... silently not committing would not be a good thing either ...

LisaAlmarode commented 6 years ago

A commit that is a no-op is not the same as a failed commit. In this kind of situation, GS server treats it as a commit that succeeded, since in fact we have successfully ensured that all changes are committed.

dalehenrich commented 6 years ago

I understand that... when using git at the command line, I have had cases where I did a commit, "knowing" that I had made changes on disk and getting an error back when I did the commit, clued me in to the fact that I was mistaken ... in some way ... in these cases, I had not done an add yet, so it wasn't an "empty commit" per se and I was glad that it was noisy when it did not have anything to commit ... this extra add step is whey the error exists ...

So, whether or not the GUI silently commits or not will depend upon how far we deviate from following the standard git command line procedure...

ericwinger commented 5 years ago

@LisaAlmarode I see this is assigned to me. If it's truly a Jadeite bug or enhancement request, can you move the bug to the Jadeite project? Else, reassign. Thanks.

LisaAlmarode commented 5 years ago

In Jadeite browsers, since you (the end-user) don't actually have the option to do an add or not, it doesn't seem like the git command line procedures are relevant. I'm not aware that Rowan itself has a did-you-do-a-git-add question; I get the same walkback-on-comit-no-changes using definitions code

Write to git does not have this problem, I can write to git when there are no changes. This is what I generally do now, so the just-in-case-commit walkback is not something I run into very much.

I think this should have attention in Rowan, not just Jadeite. While I can understand wanting to have some kind of notification, this ought to be some kind of an error message, not a walkback. The walkback is quite alarming, and the "clean repository" part of the walkback is not visible, and hard to get at. Not a big priority, however.

dalehenrich commented 5 years ago

I think this is a jadeite issue if not transfer back to Rowan...

LisaAlmarode commented 5 years ago

This still happens in 3.0.75. Haven't been doing any git commits myself lately but it is still an alarming message.

ericwinger commented 5 years ago

Because the error occurs in rowan code, the best Jadeite can do is disable the Commit to Git ... menu item if the project is not dirty in both the project list & project browser. With luck, this will suffice. Fixed in 3.0.76.

LisaAlmarode commented 3 years ago

This is not fixed, since if you make a change and undo it, you will get the walkback. If Jadeite can't handle it, then it needs to go to Rowan. It's pretty minor but it does need to get dealt with for some future GA release.

LisaAlmarode commented 3 years ago

The problem is that the commit to git will fail, if the on-disk state is the same as what was already in git. The jadeite project could be dirty due to add and remove of a space, for example.

Can Jadeite detect if the on-disk files in the repository, that would be git-checked-in, actually has changes from what is already checked in? If Jadeite can figure that out, it can enable/disable commits to avoid a walkback. E.g., if Jadeite does a two-step process of write then commit, we could insert the query in between and only commit if there were changes.

If Rowan handles the write to disk and git commit within a single operation that jadeite invokes, then Rowan is going to have to do that query, or handle the error. If that's too hard for Rowan to do, possible a new API method would be needed - Jadeite could do a write, query for changes, and then commit.

dalehenrich commented 3 years ago

@LisaAlmarode I don't know which version of Jadeite and Rowan you are looking at here, but in V3/V2.2, I think you should be getting a dialog that contains the message from the result of the commit ... if there was nothing to commit, you get a dialog telling you that there is nothing to commit ... I've got tests that handle the interaction request and check the message ... so I've guessing you are using an older version of Jadeite?

LisaAlmarode commented 1 year ago

As of 3.2.12, you first get: image then you press OK, and get: image

git log indicates it was not committed, so this second dialog is wrong. However, it's better than a walkback.

ericwinger commented 1 year ago

If Rowan determines there is nothing to commit to git, it sends a client forwarder message back to the client. However, the client forwarder is handled at a low level so the rowan services code doesn't get to intercept it. In addition, Jadeite doesn't get notified that no commit happened because RwPrjCommitTool>>commitProjectNamed:message: doesn't return the result of the git commit. Filed https://github.com/GemTalk/Rowan/issues/887 to address.