search

vermiculus / magithub

**DEPRECATED - please use Forge instead!** -- Magit-based interfaces to GitHub
GNU General Public License v3.0
579 stars 63 forks source link

Make magithub-comment-reply quote the region if it is active #257

Closed iravid closed 6 years ago

iravid commented 6 years ago

Resolves #244

iravid commented 6 years ago

I just started using Magithub and it's awesome, so thought I'd pick something off the issues list and solve it :-)

vermiculus commented 6 years ago

Can't wait to try it out, thanks!!

iravid commented 6 years ago

Sure :-) I had to context switch so I couldn’t try it out myself yet, but hopefully tonight :)

iravid commented 6 years ago

Whoops, sorry, I misread the code and this isn't doing the right thing yet.

iravid commented 6 years ago

Ok, should be good now. I'm not sure it's the cleanest way of doing this as https://stackoverflow.com/a/605931/2535374 says we should be using the (interactive "r") form to integrate the start/end points into the function arguments, but it works :-)

vermiculus commented 6 years ago

It's not working for me ☹ use-region-p is returning nil because region-active-p is nil.

Unless I'm doing something incorrectly:

  1. Place point at start of region.
  2. C-SPC to start the selection (i.e., place mark).
  3. Move point to the end of my region.
  4. Use r to call magithub-comment-reply.
iravid commented 6 years ago

It's not working for me ☹ use-region-p is returning nil because region-active-p is nil.

Unless I'm doing something incorrectly:

  1. Place point at start of region.
  2. C-SPC to start the selection (i.e., place mark).
  3. Move point to the end of my region.
  4. Use r to call magithub-comment-reply.

Hmm, strange. It's not working here too, but use-region-p and region-active-p are evaluating to t when I test them with the region active.

However, I'm using Spacemacs and activating the region using v (e.g. Visual Line in evil). Maybe that has something to do with it?

I do remember getting it working at some point ;) Let me dig into this some more.

iravid commented 6 years ago

Ahh, it's probably because I moved the call to use-region-p into the with-temp-buffer form.

iravid commented 6 years ago

Ok, give this another try :-)

iravid commented 6 years ago

Hey @vermiculus, had a chance to give this another try?

vermiculus commented 6 years ago

Sorry it's taken me a little while; work's been a little crazy 😄 I'm looking now.

vermiculus commented 6 years ago

Things are looking good! Posting a link to test that.

vermiculus commented 6 years ago

Thanks!