-
Notifications
You must be signed in to change notification settings - Fork 6
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
[Bug] Fix removal of unstaged changes on branch change #11
base: main
Are you sure you want to change the base?
[Bug] Fix removal of unstaged changes on branch change #11
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let’s add an example and then we’re good to go!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I’m not opposed to the changes in this file, they seem unrelated. I don’t necessarily think we need to remove them, but wanted to point them out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this actually was from a monkey patch that I added to my personal config. The bigger inlays match the styling of other dropdown menus. But we don't have to include it since it isn't relevant to the actual fix i'm changing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can keep this in, it doesn’t really change the intent of the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to work well! Can we add an example for it? GtGitElementExamples>>#scripterOnExpandedElementWithEditedBranch
could be a starting point for how it can be done.
If you need help or guidance, let me know!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So when adding the example, should I invoke this example in another example for using the changes?
So invoke self scripterOnExpandedElementWithEditedBranch
, then checkout to a new branch to show the changes persist?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello, apologies for the delayed answer. I think that’s the way to go, yes. Change the branch (with some changes), assert that the changes are still there.
I came across what I think to be a bug, or at the very least a non obvious interaction when using the git interface.
When a repository has unstaged changes, and we create a new branch, or switch to another branch, all unstaged changes are reverted.
While we can fix this by going into the code changes and reverting, I think the default functionality should be closer to how git normally works, which is that checking out a new branch should not remove unstaged changes, unless a force flag is passed.
This is the default behaviour of the morphic git interface in pharo as well.
Let me know what you think!