ananthakumaran / monky

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

monky-status doesn't work #1

Closed gwenhael-le-moine closed 13 years ago

gwenhael-le-moine commented 13 years ago

When I invoke monky-status I get the help message from hg in the minibuffer and the status buffer is empty.

It might be because my system's in French.

It happens with both hg 1.8.2 and 1.9

Here's the message (it says "Setting current directory: filename too long, " and then the hg help message):

apply: Setting current directory: nom de fichier trop long, Mercurial, système de gestion de sources distribué

commandes de base :

add add the specified files on the next commit annotate show changeset information by line for each file clone make a copy of an existing repository commit commit the specified files or all outstanding changes diff diff repository (or selected files) export dump the header and diffs for one or more changesets forget forget the specified files on the next commit init create a new repository in the given directory log show revision history of entire repository or files merge merge working directory with another revision pull pull changes from the specified source push push changes to the specified destination remove remove the specified files on the next commit serve start stand-alone webserver status show changed files in the working directory summary summarize working directory state update update working directory (or switch revisions) view start interactive history viewer

utiliser "hg help" pour la liste complète des commandes ou "hg -v" pour plus de détails/

EDIT: actually the status buffer is named with the whole help message...

ananthakumaran commented 13 years ago

run hg root and post the result

gwenhael-le-moine commented 13 years ago
$ hg root
/home/cycojesus/projets/cv
ananthakumaran commented 13 years ago

i can't guess what's going wrong.(i am the only person using it as of now)

can you run

M-x toggle-debug-on-error and post the details

gwenhael-le-moine commented 13 years ago

Unfortunately there is no error per-se. Only this "File name too long" in the Messages buffer. Also I get the same behavior with LANG=C emacs -Q

One thing I forgot to mention is that I'm running emacs 24 compiled from the latest bzr head... EDIT: Same behavior with Emacs 23.3.1, also launched with LANG=C and -Q

sjamaan commented 13 years ago

Monky is running hg with --pager=no, which is an unrecognised option in plain, unconfigured hg. Then it reads stdout from the process (the mercurial help message is sent here) and it uses that entire string as the new working directory. This leads to the "file name too long" message ;)

There are three problems:

The problem here is that both (I guess) cycojesus and I don't have the "pager" extension enabled. There are two ways to solve this: Either add the pager extension to the default options to ensure it's always enabled (and the --pager option recognised) like so: --config 'extensions.pager=' or pass the option differently, via a config variable: --config 'pager.pager=' (setting pager to an empty string seems to effectively disable it). config-options can always be set. If the module that understands the option is disabled, it doesn't matter; other modules just ignore that option.

I don't know how to solve the problem that stderr isn't shown, nor how to solve the problem of the exit status (I'm not that familiar with elisp's standard library)

PS: Why is this extension hosted in git and not in mercurial? :(

sjamaan commented 13 years ago

I have no idea how the hell to attach a patch-file here (god, I can't believe Github's issue tracker sucks even more than Bitbucket's!), so I'm just going to try inlining it:


diff --git a/monky.el b/monky.el
index 76ac8cb..132706f 100644
--- a/monky.el
+++ b/monky.el
@@ -34,7 +34,7 @@
   :group 'monky
   :type 'string)

-(defcustom monky-hg-standard-options '("--pager=no")
+(defcustom monky-hg-standard-options '("--config" "pager.pager=")
   "Standard options when running Hg."
   :group 'monky
   :type '(repeat string))

gwenhael-le-moine commented 13 years ago

@sjamaan explanations make perfect sense. I'm now at home on my personal laptop on which at some point I activated the pager extension and monky works.

(@sjamaan, you're supposed to:

  1. fork the project
  2. make your change(s)
  3. send a pull request here a bit much for a one-liner maybe...)
sjamaan commented 13 years ago

Yeah, for that small patch that's too much work, especially with $^%&(*% Git.

Anyway, I had a look at the Emacs docs and I think I've come up with a patch for catching the errors from the hg subprocess that mostly works. It's ugly and probably really clumsy elisp code, but like I said I'm not familiar with the emacs standard library ;)


diff --git a/monky.el b/monky.el
index 76ac8cb..7c36757 100644
--- a/monky.el
+++ b/monky.el
@@ -1220,16 +1220,22 @@ before the last command."

 ;;; Hg utils

-(defun monky-hg-insert (args)
+(defun monky-hg-insert (args tempfile)
   (apply #'process-file
         monky-hg-executable
-        nil (list t nil) nil
-        (append monky-hg-standard-options args)))
+        nil (list t tempfile) nil (append monky-hg-standard-options args)))

 (defun monky-hg-output (args)
-  (with-output-to-string
-    (with-current-buffer standard-output
-      (monky-hg-insert args))))
+  (let ((tempfile (make-temp-name "hg-errors")))
+    (unwind-protect
+        (with-output-to-string
+          (with-current-buffer standard-output
+            (unless (eql 0 (monky-hg-insert args tempfile))
+              (error (with-temp-buffer
+                       (insert-file-contents tempfile)
+                       (replace-regexp-in-string "[\r\n]*$" ""
+                                                 (buffer-string)))))))
+      (delete-file tempfile))))

 (defun monky-hg-string (&rest args)
   (monky-trim-line (monky-hg-output args)))
@@ -1414,7 +1420,7 @@ before the last command."

 (defun monky-insert-diff (file &optional status)
   (let ((p (point)))
-    (monky-hg-insert (list "diff" file))
+    (monky-hg-output (list "diff" file))
     (if (not (eq (char-before) ?\n))
        (insert "\n"))
     (save-restriction


(updated with cleaner code)

sjamaan commented 13 years ago

I wasn't able to stage any changes yet to view a diff, but I think that second hunk in the diff is probably wrong. It should still check for errors though, so maybe it should be wrapped in (with-output-to-buffer ...) or something.

sjamaan commented 13 years ago

Or perhaps make hg-insert do the error checking instead of monky-hg-output, that's probably better, come to think of it...

sjamaan commented 13 years ago

This change doesn't seem address the handling of errors. If for whatever reason another unrecognised switch is passed or some other error occurs, monky won't detect that and will again re-use the error report as the root directory.

See my earlier comment at https://github.com/ananthakumaran/monky/issues/1#issuecomment-1600412 for a patch that should fix that.

sjamaan commented 13 years ago

Of course github doesn't allow me to reopen this issue. $%^!^&

ananthakumaran commented 13 years ago

If for whatever reason another unrecognised switch is passed or some other error occurs, monky won't detect that

if it happens for some reason, then there is problem some where in the code, not in the hg-output(which runs the hg command and returns the stdout). Handling error is a separate issue i guess.

Why is this extension hosted in git and not in mercurial? :(

I use git and github more than hg and bitbucket. The reason i wrote this mode is that i am so used to magit.

sjamaan commented 13 years ago

If it happens for some reason, then there is problem some where in the code, not in the hg-output (which runs the hg command and returns the stdout).

Bugs happen, and the design of hg-output is wrong if it doesn't ever throw an error, because all information about the fact that an error occurred is lost by the time hg-output returns; at that point all you have is the stdout contents as a string. Stderr is lost.

Fail early and fail noisily; it's no shame to let hg-output raise an error if the underlying hg operation returns an error. Yes, it's the calling code that caused the error situation (or not! For example, what happens if the hg binary is missing? Or what if it's a different program than you think it is? In that case it is hg-output that caused the error), but that doesn't matter. You want to be informed of it as soon as possible.

Handling error is a separate issue i guess.

I can open another ticket if you prefer.

ananthakumaran commented 13 years ago

Bugs happen, and the design of hg-output is wrong if it doesn't ever throw an error, because all information about the fact that an error occurred is lost by the time hg-output returns; at that point all you have is the stdout contents as a string. Stderr is lost. Fail early and fail noisily; it's no shame to let hg-output raise an error if the underlying hg operation returns an error. Yes, it's the calling code that caused the error situation (or not! For example, what happens if the hg binary is missing? Or what if it's a different program than you think it is? In that case it is hg-output that caused the error), but that doesn't matter. You want to be informed of it as soon as possible

agreed.