fix filename regex for pry 0.13.1

[sdubinsky]

Fix for issue #6.

[rocky]

Does this also work on other versions? I see for example on Ruby 1.0.0 pre?

Thanks for putting in the PR.

[sdubinsky]

0.13.1 is the latest released version of pry, so I assume it'll work on 3.0.0 as well. I downloaded a copy of 3.0.0 to check, but I couldn't manage to get it working properly without conflicting with my already-installed rubies.

Based on poking through the pry repo, this change looks to have happened between versions 12 and 13, which was released in March.

[rocky]

Thanks for the information. The question, and I don't know the answer, is whether it worth the effort to support both syntaxes. After all this is just a regular expression.

[sdubinsky]

I tried (if (string-match-p "0.13.1" (shell-command-to-string "pry --version")) "^From: %s:%s" "^From: %s @ line %s") and it seems to work. I had to attach realgud:pry to the ruby-mode hook because I use chruby, and there was some weirdness with pry where I could only get the old version to load if I explicitly used bundle exec.

[Sasanidas]

I tried (if (string-match-p "0.13.1" (shell-command-to-string "pry --version")) "^From: %s:%s" "^From: %s @ line %s") and it seems to work. I had to attach realgud:pry to the ruby-mode hook because I use chruby, and there was some weirdness with pry where I could only get the old version to load if I explicitly used bundle exec.

I think this can be more generalise, if the version changes, and the regex keep the same, it will go back the the old version regex.

Maybe we can assume that the regex isn't going to change in the next 0.13.2?
If that is the case, I think it may be interesting to parse the command string, extract the number version and check if it is >= 0.13.1.

[sdubinsky]

That's a good point. I don't think we need to parse, but probably setting the current regex as the default is better.

[Sasanidas]

That's a good point. I don't think we need to parse, but probably setting the current regex as the default is better.

Seems reasonable

[rocky]

If I understand correctly either of the two suggestions is fine with me.

We could either change @ line to (?: @line\\|:) since here the regexp doesn't get too complicated, or set the regexp based on the tested pry version.

I leave it up to you all to decide which you want to do since, you all will be making the change and will have to live with the result and maintanance.

I'll also note that in some other parts of the realgud code, I had started adding a list of regexps for triggering on since it just got too difficult and ugly to try to capture everything in a single regexp. But here that doesn't seem to be the case in my opinion. But again, you all decide.

[Sasanidas]

If I understand correctly either of the two suggestions is fine with me.

We could either change @ line to (?: @line\\|:) since here the regexp doesn't get too complicated, or set the regexp based on the tested pry version.

I leave it up to you all to decide which you want to do since, you all will be making the change and will have to live with the result and maintanance.

I'll also note that in some other parts of the realgud code, I had started adding a list of regexps for triggering on since it just got too difficult and ugly to try to capture everything in a single regexp. But here that doesn't seem to be the case in my opinion. But again, you all decide.

Yes change the regex seems most interesting, document the change it's also important.
If you want to add these changes @sdubinsky, I'll merge the request.

[sdubinsky]

tested manually.

[Sasanidas]

tested manually.

Add a test to test/test-regexp-pry.el, would be nice too 👍

[sdubinsky]

Added. The line at the top says "press c-x c-e on the next line to run the tests non-interactively" but when I tried that it complained it couldn't find load-relative. I got around it by just eval-ing the buffer, but it was still strange.

[rocky]

Added. The line at the top says "press c-x c-e on the next line to run the tests non-interactively" but when I tried that it complained it couldn't find load-relative. I got around it by just eval-ing the buffer, but it was still strange.

I'm guessing that the first time you ran, GNU Emacs had not loaded load-relative which is the line or so after that comment. Eval'ing of course defined that and so when you ran the shell EMACSLOADPATH now had the information on where `load-realative' is stored

Therefore a better comment line would look like:

;; (compile (format "EMACSLOADPATH=:%s:%s:%s:%s ./autogen.sh" (file-name-directory (locate-library "loc-changes.elc")) (file-name-directory (locate-library "test-simple.elc")) (file-name-directory (locate-library "load-relative.elc")) (file-name-directory (locate-library "realgud.elc"))))

and indeed you'll see this in other (newer) repos: https://github.com/realgud/realgud-lldb/blob/master/realgud-lldb.el#L36

[rocky]

@sdubinsky would you like write access to be able to commit to this project? If so let me know.

[sdubinsky]

Write access would be nice. I also want to add a -delayed variant to this and maybe the pdb debugger too.

[rocky]

Invite sent. I don't know what:

I also want to add a -delayed variant to this and maybe the pdb debugger too.

means. Which github repositories are we talking about?

[sdubinsky]

This one, I see it's already in pdb.

What I mean is something like in trepan2: https://github.com/realgud/realgud/blob/master/realgud/debugger/trepan2/trepan2.el#L79