Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JunpeiAnzai
Copy link

@JunpeiAnzai JunpeiAnzai commented Jul 19, 2018

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
Copy link
Collaborator

cvmat commented Jul 21, 2018

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 6d10d17 and the discussion in curl/curl#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 curl/curl@aa5fc88 with the condition if(hdrcbdata->global->isatty && !hdrcbdata->config->terminal_binary_ok) may be a solution.

@JunpeiAnzai
Copy link
Author

Thank you for your explanations & suggestions.
I've reported to curl to fix this behavior. curl/curl#2774

@cvmat
Copy link
Collaborator

cvmat commented Jul 23, 2018

@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
Copy link
Contributor

@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
Copy link
Collaborator

cvmat commented Jul 28, 2018

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

twittering-update-service-configuration-sentinel: Format "nil" is not supported
3 participants