Late night fix... the two stash-message-colon-related bugs fixed.

If I run the updated test without the change in lib/git/lib.rb, it passes.

Please re-submit with a test that fails without your change.

Also, I am not sure what the "MISNAMED" part is for. I think that a stash operation is always "<some text>: <stash message>". I don't think this additional check is necessary so it is probably best to leave this logic out.

@jcouball Thanks for your review

The story behind this minor fix is me git-stashing from within emacs and this gem crashing my collab automation using it. You see, whereas stash save prefixes "On <BRANCH>: " to the message, stash store, for instance, doesn't and cannot really do it, and it appears that some integrations which manage git metadata themselves (like this gem reads the stash metadata file on its own, not through official git functionality) also allow messages with no branch prefix -- with no real problems with those stashes down the road.

At first, I misidentified the issue with the code, and also, forgot how Regexp greediness works in Ruby -- I myself never rely on particular greediness kind and make my regexes agnostic, so I will drop that part of the "fix" if you like.

Now, for the second "MISNAMED" fix (the actual crash upon "unconventionally" named stashes), I honestly didn't know what to do to stay in line with the gem's logic because it seems stash objects here include branch information -- which is not always available. So, as I wouldn't want to make a big deal out of it, accommodating for these corner cases -- just to prevent crashes. I wasn't also sure if it's acceptable to create a "problem" stash for the test through running stash store or something, since creating stashes with the gem will not reproduce the problem.

Thanks again, let me know what you think and I'll accommodate, of course.

Ok, I see what the problem is when the stash entry is created with git store. Honestly, I think that the logic in the gem (stripping the branch name) is not correct because it can not be applied in all situations. IMHO, the message with the branch name should be returned as is without removing the branch name but this will change existing behavior which existing users may rely on.

To maintain backward compatibility for 2.x version, I think we have to compromise with your case. Since it is a pretty rare case (I'd guess), it seems like the best option for 2.x. I propose that in your case where the stash is created with git stash store -m "testing: my stash" <commit>, Lib#stashes_all will need to return "my stash" until this behavior can be changed in 3.x. At least this does not raise an error.

I opened #750 to address this issue for 3.x.

Below are my proposed tests. The 1st test below replaces the existing test (before your changes). The 3rd test fails and is awaiting your implementation change. Note that in the 4th test shows the compromise described above.

  test 'Git::Lib#stashes_all' do
    in_bare_repo_clone do |g|
      assert_equal(0, g.branch.stashes.size)
      new_file('test-file1', 'blahblahblah1')
      new_file('test-file2', 'blahblahblah2')
      assert(g.status.untracked.assoc('test-file1'))

      g.add

      assert(g.status.added.assoc('test-file1'))

      g.branch.stashes.save('testing-stash-all')

      # puts `cat .git/logs/refs/stash`
      # 0000000000000000000000000000000000000000 b9b008cd179b0e8c4b8cda35bac43f7011a0836a James Couball <jcouball@yahoo.com> 1729463252 -0700   On master: testing-stash-all

      stashes = assert_nothing_raised { g.lib.stashes_all }

      expected_stashes = [
        [0, 'testing-stash-all']
      ]

      assert_equal(expected_stashes, stashes)
    end
  end

  test 'Git::Lib#stashes_all - stash message has colon' do
    in_bare_repo_clone do |g|
      assert_equal(0, g.branch.stashes.size)
      new_file('test-file1', 'blahblahblah1')
      new_file('test-file2', 'blahblahblah2')
      assert(g.status.untracked.assoc('test-file1'))

      g.add

      assert(g.status.added.assoc('test-file1'))

      g.branch.stashes.save('saving: testing-stash-all')

      binding.irb

      # puts `cat .git/logs/refs/stash`
      # 0000000000000000000000000000000000000000 b9b008cd179b0e8c4b8cda35bac43f7011a0836a James Couball <jcouball@yahoo.com> 1729463252 -0700   On master: saving: testing-stash-all

      stashes = assert_nothing_raised { g.lib.stashes_all }

      expected_stashes = [
        [0, 'saving: testing-stash-all']
      ]

      assert_equal(expected_stashes, stashes)
    end
  end

  test 'Git::Lib#stashes_all -- git stash message with no branch and no colon' do
    in_temp_dir do
      `git init`
      `echo "hello world" > file1.txt`
      `git add file1.txt`
      `git commit -m "First commit"`
      `echo "update" > file1.txt`
      commit = `git stash create "stash message"`.chomp
      # Create a stash with this message: 'custom message'
      `git stash store -m "custom message" #{commit}`

      # puts `cat .git/logs/refs/stash`
      # 0000000000000000000000000000000000000000 0550a54ed781eda364ca3c22fcc46c37acae4bd6 James Couball <jcouball@yahoo.com> 1729460302 -0700   custom message

      git = Git.open('.')

      stashes = assert_nothing_raised { git.lib.stashes_all }

      expected_stashes = [
        [0, 'custom message']
      ]

      assert_equal(expected_stashes, stashes)
    end
  end

  test 'Git::Lib#stashes_all -- git stash message with no branch and explicit colon' do
    in_temp_dir do
      `git init`
      `echo "hello world" > file1.txt`
      `git add file1.txt`
      `git commit -m "First commit"`
      `echo "update" > file1.txt`
      commit = `git stash create "stash message"`.chomp
      # Create a stash with this message: 'custom message'
      `git stash store -m "testing: custom message" #{commit}`

      # puts `cat .git/logs/refs/stash`
      # 0000000000000000000000000000000000000000 eadd7858e53ea4fb8b1383d69cade1806d948867 James Couball <jcouball@yahoo.com> 1729462039 -0700   testing: custom message

      git = Git.open('.')

      stashes = assert_nothing_raised { git.lib.stashes_all }

      expected_stashes = [
        [0, 'custom message']
      ]

      assert_equal(expected_stashes, stashes)
    end
  end

Also, please squash your commits into one commit with a message that is suitable to show up in the release notes.

@jcouball done and done

I also hope that you don't mind I've "Dockerised" the tests; I do all my development containerised within a remote ("self server") dev env.

To note, I usually call the testing entry point bin/test, but it was already taken in this project, so I went for bin/tests by the name of the tests directory.

You're welcome to have a look at my "self server" "decentralized" git development demo at https://medium.com/@costa/git-decentralized-a25f00fd2955

@costa Thank you for your contribution of the bug fix!

Running tests in docker is neat. Thanks! This is something that might be built upon for testing against different versions of git.

I will follow up with a PR for the following:

• I will rename bin/tests to bin/test-in-docker
• I will try make the same command line args work with bin/test-in-docker. For example, bin/test test_stashes just runs the tests from tests/units/test_stashes).
• I will remove the duplicated test from test_stashes.

Hopefully, I will finish these changes soon and do a new release sometime today.

@costa

I addressed the feedback above in #751 and published 3.2.1 of the git gem to rubygems.org.

See the change log in the Github release for 3.2.1.

Thank you. If you may be interested in my container-based and git-based methodology tooling, e.g. a spec helper:
https://gist.github.com/costa/1f7a77d0949a1ee1f612fcd35b4fc23e
-- used in conjunction with:
https://gist.github.com/costa/f9ae7184ca254b25f98872ae7a170358
-- I'll be glad to notify you by email when I publish some relevant materials.