ananthakumaran / monky

Magit for Hg
http://ananthakumaran.in/monky/index.html
GNU General Public License v3.0
154 stars 30 forks source link

Add monky-histedit. #93

Open ncalexan opened 5 years ago

ncalexan commented 5 years ago

I've been using this locally for a very long time, so I figured I should push it upstream. I'd like to follow on with changes to monky-status such that monky recognizes in-flight histedit, but that's a bigger project: https://github.com/ananthakumaran/monky/issues/92.

Wilfred commented 5 years ago

Awesome, this is a much needed feature! :+1:

This doesn't work when I try it. The mercurial process hangs. The contents of *monky-process* are:

$ hg --config diff.git=Off --config ui.merge=:merge histedit --rev bd8674e0d48bf80a311a0ecff5acef126c45bf83
[Using open mode]

Open and visual must be used interactively

any idea what's wrong?

ncalexan commented 5 years ago

Open and visual must be used interactively

A quick Google suggests that this is an error message from vi, which suggests your $EDITOR is not sensible from within Emacs. Can you try to make $EDITOR be emacsclient in some way?

I would be happy to try to force $EDITOR from within Emacs, since this is likely to be a problem for many, many people. (And indeed, I've had to make monky use certain settings to avoid launching default merge tools, etc.)

Wilfred commented 5 years ago

Yep, I've not set $EDITOR. What have you set it to?

I think the proper solution here would be to use with-editor, similar to the discussion in #83.

ncalexan commented 5 years ago

Yep, I've not set $EDITOR. What have you set it to?

I'm on macOS, so I have a non-trivial e script that does emacsclient and a bunch of focus-related hacks.

I think the proper solution here would be to use with-editor, similar to the discussion in #83.

Yes, I concur. Are we willing to take with-editor as a hard dependency of monky? Personally, I think it's a sensible dependency, but every dependency costs...

Wilfred commented 5 years ago

I'm very willing to take with-editor as a dependency, I think it's a great fit and it's been battle tested in magit.

ncalexan commented 5 years ago

I'm very willing to take with-editor as a dependency, I think it's a great fit and it's been battle tested in magit.

Hey! I finally updated monky and rebased this patch. I don't really know how to declare a dependency (like, a real dependency, with ELPA getting it automatically, etc).

We can:

  1. make with-editor actually (require 'with-editor), forcing folks to install it. That seems harsh.
  2. make histedit require with-editor. Then only real users of histedit break.
  3. make with-editor an optional dependency. That might be a little awkward 'cuz I think we also want to use with-editor-set-process-filter, but it could be done.

@Wilfred your thoughts?

Wilfred commented 5 years ago

I'm entirely comfortable with depending on with-editor directly. Conditionally depending on packages is fiddly in elisp and I think the vast majority of users will install from MELPA where it all just works.

ncalexan commented 4 years ago

I'm entirely comfortable with depending on with-editor directly. Conditionally depending on packages is fiddly in elisp and I think the vast majority of users will install from MELPA where it all just works.

@Wilfred I finally had reason to address this for myself, and I discovered the package dependency incantations are perhaps simpler now than before. However, I don't know how to test that they would actually work, in the sense that selecting to install monky will install with-editor first. Do you know a way to test that?

In any case, the actual functionality works for me locally. Thanks!

melutovich commented 1 year ago

@Wilfred @ncalexan Where are we holding with this? (As you can guess by my recent pings around here, I'm looking at monky; for a new server, dvc does not work on just install, so I'm seeing my other options. @ncalexan I see your fork has more commits than this original)