"reword" in interactive rebasing no longer work #497

Closed
YorkZ opened this Issue Oct 17, 2012 · 6 comments

Projects

None yet

2 participants

@YorkZ
YorkZ commented Oct 17, 2012

This works in "dc482c5" but not in the latest commit. Here is the steps toreproduce the problem:

  1. In magit status buffer, type "l".
  2. Type "l" again.
  3. In "magit-log" buffer, select any commit.
  4. Type "E" to start interactive rebase.
  5. In "git-rebase-todo" buffer, select any commit.
  6. Change "pick" to "r" and type "C-x #"
  7. An empty buffer named "$@" would show up which is wrong. It should bring up a buffer with the existing commit message to let you edit on.
@vanicat
vanicat commented Oct 17, 2012

You probably should use magit's rewrite posibilities instead of interactive rebase.

That said, the fact that the emacsclient is called correctly for the "git-rebase-todo" buffer, but not for the commit message let me belive that the problem is in git not magit.

Do you confirm that you are using Windows?

@YorkZ
YorkZ commented Oct 18, 2012

Firstly yes I'm using Windows. Secondly, I don't think the problem is in git because I have been using interactive rebasing every day for years. Interactive rebasing is something I'm doing hundreds of times a day, I can't live without it.

@YorkZ
YorkZ commented Oct 18, 2012

I just test it and this patch works on my system. Do think it is appropriate to apply this patch?

@@ -5617,9 +5617,13 @@ Return values:
                       (magit-section-info section)))
          (old-editor (getenv "GIT_EDITOR")))
     (if (executable-find "emacsclient")
-        (setenv "GIT_EDITOR" (concat (executable-find "emacsclient")
-                                     " -s " server-name))
-        (message "Cannot find emacsclient, using default git editor, please check you PATH"))
+        (setenv "GIT_EDITOR"
+                (if (eq system-type 'windows-nt)
+                    (executable-find "emacsclient")
+                  (concat (executable-find "emacsclient")
+                          " -s " server-name)))
+      (message "Cannot find emacsclient, using default git editor, please
+check you PATH"))
     (unwind-protect
         (magit-run-git-async "rebase" "-i"
                              (or (and commit (concat commit "^"))
@vanicat
vanicat commented Oct 18, 2012

Magit's rewrite do exactly the same as git's interactive rebase, but completely inside git. You should give it a try.

We still need to solve the bug. I might use your patch but it let a bug: if someone don't use the default server name on windows, emacsclient won't be called with this alternate server name.

Something like that would be better: we still have the problem on windows, but the user is notified. Note that I didn't had time to test it yet.

      (if (executable-find "emacsclient")
 -        (setenv "GIT_EDITOR" (concat (executable-find "emacsclient")
 -                                     " -s " server-name))
 +        (setenv "GIT_EDITOR"
 +                (if (string= server-name "server")
 +                    (executable-find "emacsclient")
 +                    (if (system-type 'windows-nt)
 +                        (progn
 +                           (message "We don't know how to deal with non-default server name on windows")
 +                           ())
 +                        (concat (executable-find "emacsclient")
 +                                " -s " server-name))))
         (message "Cannot find emacsclient, using default git editor, please check you PATH"))
      (unwind-protect
          (magit-run-git-async "rebase" "-i"
                               (or (and commit (concat commit "^"))

Now I've to find a way to check if there is indeed a misfeature in git...

@vanicat vanicat was assigned Apr 20, 2013
@YorkZ
YorkZ commented May 2, 2013

Sorry for bringing up this 7 month old issue, but it is still a problem which had never been addressed yet. Hard to believe that I seemed to be the only person who uses Magit on Windows for interactive rebase. Both my patch and Vanicat's
patch worked for me on my Windows 7 system. Below is the patch that I'm currently using, it is basically Vanicat's patch but corrected a typo:

@@ -6345,9 +6345,16 @@ Return values:
                       (magit-section-info section)))
          (old-editor (getenv "GIT_EDITOR")))
     (if (executable-find "emacsclient")
-        (setenv "GIT_EDITOR" (concat (executable-find "emacsclient")
-                                     " -s " server-name))
-        (message "Cannot find emacsclient, using default git editor, please check your PATH"))
+        (setenv "GIT_EDITOR"
+                (if (string= server-name "server")
+                    (executable-find "emacsclient")
+                  (if (eq system-type 'windows-nt)
+                      (progn
+                        (message "We don't know how to deal with non-default server name on windows")
+                        ())
+                    (concat (executable-find "emacsclient")
+                            " -s " server-name))))
+      (message "Cannot find emacsclient, using default git editor, please check your PATH"))
     (unwind-protect
         (magit-run-git-async "rebase" "-i"
                              (or (and commit (concat commit "^"))

I would appreciate it if some one (Vanicat maybe) could use this patch (or anything that fixes the original problem).

Thanks,

York

@vanicat vanicat added a commit that closed this issue May 2, 2013
@vanicat vanicat Using just emaclient as editor if server-name is the default value.
Otherwise there are problem on windows (Closes #497)
There is still a problem with non standard server name on windows, but
this commit fix most common case.
52c57e7
@vanicat vanicat closed this in 52c57e7 May 2, 2013
@vanicat
vanicat commented May 2, 2013

Sorry for the delay, I was waiting for positive feedback then forgot about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment