hayamiz / twittering-mode

An Emacs major mode for Twitter
http://twmode.sourceforge.net/
545 stars 92 forks source link

Fix the regex of content-type in order to accept ANSI color code #149

Open JunpeiAnzai opened 5 years ago

JunpeiAnzai commented 5 years ago

Since curl version 7.61.0, the keys of response header are shown bold, in other words, response header contains ANSI color code [1]. For example, \x1b[1mcontent-type\x1b[21m instead of content-type.

This PR fixes regex for detection of content-type, and resolves #148 .

[1] https://curl.haxx.se/changes.html

cvmat commented 5 years ago

The current version of twittering-mode invokes curl with the option --output - to allow curl to output a HTTP response as binary. In detail, see 6d10d1765a7b4de4c723395c8a2200a1649beeb0 and the discussion in https://github.com/curl/curl/pull/2538 . I think that the problem is a bug of curl because decorations for human readability are meaningless in the case where the command may output binary data. The curl program should output a response as is without any decoration when it outputs binary data.

The problem is due to inconsistency of how to determine modes related to output.

I think that replacing the condition expression if(hdrcbdata->global->isatty) in the commit https://github.com/curl/curl/pull/2538/commits/aa5fc886fe049383c13285f782f970cd0230d275 with the condition if(hdrcbdata->global->isatty && !hdrcbdata->config->terminal_binary_ok) may be a solution.

JunpeiAnzai commented 5 years ago

Thank you for your explanations & suggestions. I've reported to curl to fix this behavior. https://github.com/curl/curl/pull/2774

cvmat commented 5 years ago

@JunpeiAnzai The cause of the problem may be in my code. I am sorry that my investigation of twittering-mode was not enough. Can I ask you to check whether the below patch solves the problem?

diff -r 6d7303d1a7e1 twittering-mode.el
--- a/twittering-mode.el        Mon May 07 23:21:48 2018 +0900
+++ b/twittering-mode.el        Mon Jul 23 11:16:59 2018 +0900
@@ -952,7 +952,9 @@

 This function is the same as `start-process' except that SENTINEL must
 be invoked when the process is successfully started."
-  (let ((proc (apply 'start-process name buffer program args)))
+  (let* (;; Ensure that the process communicates with a pipe instead of a pty.
+        (process-connection-type nil)
+        (proc (apply 'start-process name buffer program args)))
     (when (and proc (functionp sentinel))
       (if (twittering-process-alive-p proc)
          (set-process-sentinel proc sentinel)
multiSnow commented 5 years ago

@cvmat The patch works. Besides, curl has a new option --no-styled-output added in 7.61.0 that turn off style with the output.

cvmat commented 5 years ago

@multiSnow Thank you for confirmation. I have committed the patch as fbdd433de60f0e83f55a6560685409400216a42c.

As you said, this problem can be avoided by the new option --no-styled-output. But it causes an error with curl older than 7.61.0. I personally want to avoid codes that depend on specific versions of external programs, if possible.