Refactor env, source, and exec

[eliangcs]

Refactor PR #73.

• Cover more edge cases
• Add more test cases
• Make code more Pythonic
• Abandon JSON in favor of "http-prompt script" for persistent context
• Fix an autocomplete bug

@fogine Would you like to review it?

[fogine]

Neat work @eliangcs !

I've stumbled upon a bug with cmd output redirection lexer definition:

https://api.github.com> get >> /tmp/test   # the file path is highlighted as syntax err

I believe this bug has been already present in my original PR. It's specific to the action commands output redirection.

I've also noticed few other issues with command output redirection:

• when appending to a file, no new line character is appended before attaching new data. if this change is intentional, we should come up with an alternative way how to have the option of inserting the new line character
• for actions commands (eg.: get > /tmp/file), a data redirected are not same as when cmd output is written to stdout. In addition, a data written to a file are minified (new line characters are striped). For example, when I did GET request to "https://api.github.com" I got THIS to stdout and THIS written to a file. I'd expect the exact same output I get to stdout to be written to a file.

[eliangcs]

@fogine Thanks for finding the bug! I just pushed several patches:

• Fix the get >> /tmp/test lexer bug
• Make sure we always append a newline char whenever we output something
• Use a different grammar on Windows to avoid using backslash as an escape char in a file path
• Update README

As for

I'd expect the exact same output I get to stdout to be written to a file.

The change is actually intentional, so that it is consistent with HTTPie's redirected output. See this example:

$ http http://httpbin.org/user-agent
HTTP/1.1 200 OK
Access-Control-Allow-Credentials: true
Access-Control-Allow-Origin: *
Connection: keep-alive
Content-Length: 35
Content-Type: application/json
Date: Fri, 16 Sep 2016 17:15:08 GMT
Server: nginx

{
    "user-agent": "HTTPie/0.9.6"
}

$ http http://httpbin.org/user-agent > out.txt
$ cat out.txt
{
  "user-agent": "HTTPie/0.9.6"
}

[fogine]

@eliangcs , thanks for spending the time on this recently!

Make sure we always append a newline char whenever we output something

This actually still does not work for action commands. It's because we let httpie directly write to a stream here...

The change is actually intentional, so that it is consistent with HTTPie's redirected output.

I see. That makes sense... Just for possible readers, the "issue" is solved by explicitly defining --pretty=format and --print=hb options before/while making a request..

@eliangcs , I'm interested in seeing how have you solved the problem with stripping ANSI color codes. I see that the actual stripping isn't necessary anymore... but I can't find a code which handles this... could you point me to the correct direction, please?

Thanks!

[eliangcs]

@fogine I don't think a newline char at the end of an action command output is necessary. We want to respect what the HTTP server sends to a client. The design goal is to be consistent with HTTPie.

The output redirection of an action command is handled by httpie.core.main itself. See https://github.com/eliangcs/http-prompt/blob/45e0aa3cddca94c113198fba90ab3ce50f10abec/http_prompt/execution.py#L388. I passed self.output (the current output stream) to httpie.core.main and let it deal with ANSI color codes.

[fogine]

I don't think a newline char at the end of an action command output is necessary. We want to respect what the HTTP server sends to a client. The design goal is to be consistent with HTTPie.

Hmm, good point @eliangcs , however I don't think this design decision is gonna be much useful to users of http-prompt when doing append operation eg.: localhost> get >> /tmp/test ...
We can still alter the behavior later I guess...

[eliangcs]

@fogine Normally, a server should return a newline char at the end if the response is in text format. What use case are you thinking about?

[fogine]

Normally, a server should return a newline char at the end if the response is in text format.

@eliangcs , that applies only for text/plain response data... Anything else like application/json is most probably not gonna have new line char at the end...
I think only case where it makes sense to not insert new line character when appending data to a file is when dealing with binary data...
In most use cases, users are going to append a text response (in any form eg.: json,xml,plain) to a file. In my opinion, it simply doesn't feel right to not split each record with a new line character... It decreases readability and complicates possible parsing of file with responses...

Do you think there are other advantages in the current behavior except the consistency ?

Ps.: curl also appends a new line character when doing eg.:

> curl -X GET https://api.github.com >> /tmp/test

[eliangcs]

@fogine Besides consistency, I can't think of a good way to tell if a response is text or binary without looking at the content. The best we can do is guess from the Content-Type header. There are so many Content-Types and edge cases we'll have to deal with. What if the response already has a newline at the end? What if it has no Content-Type header? What if the Content-Type is application/json but the body is actually binary data?

What's your opinion on this?

[fogine]

@eliangcs , what if we approached this from opposite side? if we were to append to a file, we would have done something among these steps:

Is the file we're appending to empty (or does not exist) ? if so, just write data we've got to the file, otherwise try to determine if the file we're appending to is either binary or text file. if we append to a "text file", append a new line character to the file before appending actual request data.

Any thoughts? I might be missing something...
Edit: We'd still need to examine some data though...

[eliangcs]

@fogine how do you define a "text file"? By checking its content-type or content?

[fogine]

By checking its content-type or content?

@eliangcs , that was my thinking... I think this way it would be least error-prone... less exceptions to handle...

[eliangcs]

@fogine That sounds ad-hoc, and I'm trying to avoid ad-hoc code. I guess we can do it when there are more feature requests like this and do it as a user configurable option. Thanks for the advice!

[fogine]

No problem, thanks for your time @eliangcs while discussing this. I've not found any other bugs in implemented features... so it's good to go I guess.