ronantakizawa/codereview-bench huggingface dataset:
Repository: httpie/httpie
File: tests/test_output.py (Python)

Code before:
import argparse
from pathlib import Path
from unittest import mock

import json
import os
import io
from urllib.request import urlopen

import pytest
import requests
import responses

from httpie.cli.argtypes import (
    PARSED_DEFAULT_FORMAT_OPTIONS,
    parse_format_options,
)
from httpie.cli.definition import parser
from httpie.constants import UTF8
from httpie.output.formatters.colors import get_lexer
from httpie.status import ExitStatus
from .utils import COLOR, CRLF, HTTP_OK, MockEnvironment, http

JSON_XXSI_PREFIXES = (")]}',\n", ")]}',", 'while(1);', 'for(;;)', ')', ']', '}')


@pytest.mark.parametrize('stdout_isatty', [True, False])
def test_output_option(tmp_path, httpbin, stdout_isatty):
    output_filename = tmp_path / 'test_output_option'
    url = httpbin + '/robots.txt'

    r = http('--output', str(output_filename), url,
             env=MockEnvironment(stdout_isatty=stdout_isatty))
    assert r == ''

    expected_body = urlopen(url).read().decode()
    actual_body = output_filename.read_text(encoding=UTF8)

    assert actual_body == expected_body


class TestQuietFlag:

    @pytest.mark.parametrize('argument_name', ['--quiet', '-q'])
    def test_quiet(self, httpbin, argument_name):
        env = MockEnvironment(
            stdin_isatty=True,
            stdout_isatty=True,
            devnull=io.BytesIO()

Review comment (suggestion):
```suggestion
TEST_JSON_XXSI_PREFIXES = (")]}',\n", ")]}',", 'while(1);', 'for(;;)', ')', ']', '}')
```

(To make it clear these’re the ones we test, no an exhaustiveve list)

Code after:
import argparse
from pathlib import Path
from unittest import mock

import json
import os
import io
from urllib.request import urlopen

import pytest
import requests

from httpie.cli.argtypes import (
    PARSED_DEFAULT_FORMAT_OPTIONS,
    parse_format_options,
)
from httpie.cli.definition import parser
from httpie.constants import UTF8
from httpie.output.formatters.colors import get_lexer
from httpie.status import ExitStatus
from .utils import COLOR, CRLF, HTTP_OK, MockEnvironment, http


@pytest.mark.parametrize('stdout_isatty', [True, False])
def test_output_option(tmp_path, httpbin, stdout_isatty):
    output_filename = tmp_path / 'test_output_option'
    url = httpbin + '/robots.txt'

    r = http('--output', str(output_filename), url,
             env=MockEnvironment(stdout_isatty=stdout_isatty))
    assert r == ''

    expected_body = urlopen(url).read().decode()
    actual_body = output_filename.read_text(encoding=UTF8)

    assert actual_body == expected_body


class TestQuietFlag:

    @pytest.mark.parametrize('argument_name', ['--quiet', '-q'])
    def test_quiet(self, httpbin, argument_name):
        env = MockEnvironment(
            stdin_isatty=True,
            stdout_isatty=True,
            devnull=io.BytesIO()
        )
        r = http(argument_name, 'GET', httpbin.url + '/get', env=env)
        assert env.stdout is env.devnull
Repository: httpie/httpie
File: httpie/output/utils.py (Python)

Code before:
import json
from typing import Generator, Tuple


def load_json(data: str) -> Tuple[str, json.JSONDecoder]:
    """Simple JSON loading from bodies found in `data`.
    See :func:`find_json_actual_body_and_prefix()` for how bodies are detected.

    """
    for prefix, body in find_json_actual_body_and_prefix(data):
        try:
            return prefix, json.loads(body)
        except ValueError:
            pass
    raise ValueError('Invalid JSON')


def find_json_actual_body_and_prefix(data: str) -> Generator[Tuple[str, str], None, None]:
    """Find potential JSON bodies derived from `data`.

    Sometimes the JSON body is prefixed with a XSSI magic string, specific to the server.
    In order to be able to display well-formatted JSON, that method will try several
    approaches to find the actual body of the JSON.

    Yield tuples (data prefix, actual JSON body).

    """
    # First, the full data.
    yield '', data


Review comment (suggestion):
The name’s too generic, how about:

```suggestion
def load_prefixed_json(data: str) -> Tuple[str, json.JSONDecoder]:
```

Code after:
import json
import re
from typing import Tuple

from .lexers.json import PREFIX_REGEX


def load_prefixed_json(data: str) -> Tuple[str, json.JSONDecoder]:
    """Simple JSON loading from `data`.

    """
    # First, the full data.
    try:
        return '', json.loads(data)
    except ValueError:
        pass

    # Then, try to find the start of the actual body.
    data_prefix, body = parse_prefixed_json(data)
    try:
        return data_prefix, json.loads(body)
    except ValueError:
        raise ValueError('Invalid JSON')


def parse_prefixed_json(data: str) -> Tuple[str, str]:
    """Find the potential JSON body from `data`.

    Sometimes the JSON body is prefixed with a XSSI magic string, specific to the server.
    Return a tuple (data prefix, actual JSON body).
Repository: httpie/httpie
File: httpie/output/utils.py (Python)

Code before:
import json
from typing import Generator, Tuple


def load_json(data: str) -> Tuple[str, json.JSONDecoder]:
    """Simple JSON loading from bodies found in `data`.
    See :func:`find_json_actual_body_and_prefix()` for how bodies are detected.

    """
    for prefix, body in find_json_actual_body_and_prefix(data):
        try:
            return prefix, json.loads(body)
        except ValueError:
            pass
    raise ValueError('Invalid JSON')


def find_json_actual_body_and_prefix(data: str) -> Generator[Tuple[str, str], None, None]:
    """Find potential JSON bodies derived from `data`.

    Sometimes the JSON body is prefixed with a XSSI magic string, specific to the server.
    In order to be able to display well-formatted JSON, that method will try several
    approaches to find the actual body of the JSON.

    Yield tuples (data prefix, actual JSON body).

    """
    # First, the full data.
    yield '', data

    # Then, try to find the start of the actual body.
    pos = {data.find('{'), data.find('[')}
    # `data.find(char)` return `-1` when `char` is not present in `data`, let's remove
    # `-1` from potential positions to be able to use `min(pos)` later.
    pos -= {-1}
    if pos:
        prefix = data[:min(pos)]
        body = data[min(pos):]
        yield prefix, body


Review comment (suggestion):
Then we can have:
```suggestion
def parse_prefixed_json(data: str) -> Generator[Tuple[str, str], None, None]:
```

Code after:
import json
import re
from typing import Tuple

from .lexers.json import PREFIX_REGEX


def load_prefixed_json(data: str) -> Tuple[str, json.JSONDecoder]:
    """Simple JSON loading from `data`.

    """
    # First, the full data.
    try:
        return '', json.loads(data)
    except ValueError:
        pass

    # Then, try to find the start of the actual body.
    data_prefix, body = parse_prefixed_json(data)
    try:
        return data_prefix, json.loads(body)
    except ValueError:
        raise ValueError('Invalid JSON')


def parse_prefixed_json(data: str) -> Tuple[str, str]:
    """Find the potential JSON body from `data`.

    Sometimes the JSON body is prefixed with a XSSI magic string, specific to the server.
    Return a tuple (data prefix, actual JSON body).

    """
    matches = re.findall(PREFIX_REGEX, data)
    data_prefix = matches[0] if matches else ''
    body = data[len(data_prefix):]
    return data_prefix, body
Repository: httpie/httpie
File: httpie/output/formatters/json.py (Python)

Code before:

        """
        from ..utils import find_json_actual_body_and_prefix
        for prefix, body in find_json_actual_body_and_prefix(data):
            try:
                return prefix, json.loads(body)
            except ValueError:
                pass
        raise ValueError('Invalid JSON')

    def format_body(self, body: str, mime: str) -> str:
        maybe_json = [
            'json',
            'javascript',
            'text',
        ]
        if (self.kwargs['explicit_json']
                or any(token in mime for token in maybe_json)):
            try:
                prefix, obj = self._load_json(body)
            except ValueError:
                pass  # Invalid JSON, ignore.
            else:
                # Indent, sort keys by name, and avoid
                # unicode escapes to improve readability.
                body = prefix + '\n' if prefix else ''
                body += json.dumps(
                    obj=obj,
                    sort_keys=self.format_options['json']['sort_keys'],
                    ensure_ascii=False,
                    indent=self.format_options['json']['indent']
                )
        return body


Review comment (suggestion):
I’d get rid of the extra `\n`. Without it, it will be easier to understand that it‘s the prefix. The prefix may otherwise look like some random noise if you don’t expect it. 

Code after:
import json

from ...plugins import FormatterPlugin


class JSONFormatter(FormatterPlugin):

    def __init__(self, **kwargs):
        super().__init__(**kwargs)
        self.enabled = self.format_options['json']['format']

    def format_body(self, body: str, mime: str) -> str:
        maybe_json = [
            'json',
            'javascript',
            'text',
        ]
        if (self.kwargs['explicit_json']
                or any(token in mime for token in maybe_json)):
            from ..utils import load_prefixed_json
            try:
                data_prefix, json_obj = load_prefixed_json(body)
            except ValueError:
                pass  # Invalid JSON, ignore.
            else:
                # Indent, sort keys by name, and avoid
                # unicode escapes to improve readability.
                body = data_prefix + json.dumps(
                    obj=json_obj,
                    sort_keys=self.format_options['json']['sort_keys'],
                    ensure_ascii=False,
                    indent=self.format_options['json']['indent']
                )
        return body
Repository: httpie/httpie
File: httpie/output/lexers/json.py (Python)

Code before:
    def get_tokens_unprocessed(self, text):
        """Parse JSON data."""

        in_body_prefix = False

        for index, token, value in super().get_tokens_unprocessed(text):
            if token is Token.Punctuation and self.body_prefix_sep in value:
                # `value` can be a single character or a sequence of punctuation characters.
                # We need to iterate over the whole `value` to properly handle the body prefix
                # separator from actual JSON punctuation characters.
                for char in value:
                    if char == self.body_prefix_sep:
                        # Body prefix.
                        in_body_prefix = not in_body_prefix
                    elif in_body_prefix:
                        # Punctuation character part of the body prefix.
                        yield index, self.body_prefix_token, char
                    else:
                        # Punctuation character outside the body prefix.
                        yield index, token, char
            elif in_body_prefix:
                # `value` is part of the body prefix.
                yield index, self.body_prefix_token, value
            else:
                # Outside the body prefix.
                yield index, token, value


Review comment (refactor):
You can just have a simple lexer that delegates to the JSON one anything after the prefix (needs some testing). No additional treatment for the prefix is needed when it comes to colors, so you can simplify `ColorFormatter` again.

```python
from pygments.lexer import using, RegexLexer

class EnhancedJsonLexer(RegexLexer):
    flags = re.IGNORECASE | re.DOTALL
    tokens = {
        'root': [
            (r'[^{\[]+', Token.Error),  # Prefix
            (r'.+', using(JsonLexer)),
        ],
    }
```

Code after:
import re

from pygments.lexer import bygroups, using, RegexLexer
from pygments.lexers.data import JsonLexer
from pygments.token import Token

PREFIX_TOKEN = Token.Error
PREFIX_REGEX = r'[^{\["]+'


class EnhancedJsonLexer(RegexLexer):
    """
    Enhanced JSON lexer for Pygments.

    It adds support for eventual data prefixing the actual JSON body.

    """
    name = 'JSON'
    flags = re.IGNORECASE | re.DOTALL
    tokens = {
        'root': [
            # Eventual non-JSON data prefix followed by actual JSON body.
            # FIX: data prefix + number (integer or float) are not correctly handled.
            (
                fr'({PREFIX_REGEX})' + r'((?:[{\["]|true|false|null).+)',
                bygroups(PREFIX_TOKEN, using(JsonLexer))
            ),
            # JSON body.
            (r'.+', using(JsonLexer)),
        ],
    }
Repository: httpie/httpie
File: httpie/output/utils.py (Python)

Code before:

def load_json(data: str) -> Tuple[str, json.JSONDecoder]:
    """Simple JSON loading from bodies found in `data`.
    See :func:`find_json_actual_body_and_prefix()` for how bodies are detected.

    """
    for prefix, body in find_json_actual_body_and_prefix(data):
        try:
            return prefix, json.loads(body)
        except ValueError:
            pass
    raise ValueError('Invalid JSON')


def find_json_actual_body_and_prefix(data: str) -> Generator[Tuple[str, str], None, None]:
    """Find potential JSON bodies derived from `data`.

    Sometimes the JSON body is prefixed with a XSSI magic string, specific to the server.
    In order to be able to display well-formatted JSON, that method will try several
    approaches to find the actual body of the JSON.

    Yield tuples (data prefix, actual JSON body).

    """
    # First, the full data.
    yield '', data

    # Then, try to find the start of the actual body.
    pos = {data.find('{'), data.find('[')}
    # `data.find(char)` return `-1` when `char` is not present in `data`, let's remove
    # `-1` from potential positions to be able to use `min(pos)` later.
    pos -= {-1}
    if pos:
        prefix = data[:min(pos)]
        body = data[min(pos):]
        yield prefix, body


Review comment (suggestion):
This should not be done here.  `load_prefixed_json()` should try to parse the whole string on its own before calling `parse_prefixed_json()`.

Code after:

from .lexers.json import PREFIX_REGEX


def load_prefixed_json(data: str) -> Tuple[str, json.JSONDecoder]:
    """Simple JSON loading from `data`.

    """
    # First, the full data.
    try:
        return '', json.loads(data)
    except ValueError:
        pass

    # Then, try to find the start of the actual body.
    data_prefix, body = parse_prefixed_json(data)
    try:
        return data_prefix, json.loads(body)
    except ValueError:
        raise ValueError('Invalid JSON')


def parse_prefixed_json(data: str) -> Tuple[str, str]:
    """Find the potential JSON body from `data`.

    Sometimes the JSON body is prefixed with a XSSI magic string, specific to the server.
    Return a tuple (data prefix, actual JSON body).

    """
    matches = re.findall(PREFIX_REGEX, data)
    data_prefix = matches[0] if matches else ''
    body = data[len(data_prefix):]
    return data_prefix, body
Repository: httpie/httpie
File: tests/test_json.py (Python)

Code before:
import json

import pytest
import responses

from httpie.cli.constants import PRETTY_MAP
from httpie.output.formatters.colors import ColorFormatter

from .utils import MockEnvironment, http, URL_EXAMPLE

TEST_JSON_XXSI_PREFIXES = (r")]}',\n", ")]}',", 'while(1);', 'for(;;)', ')', ']', '}')
TEST_JSON_VALUES = ({}, {'a': 0, 'b': 0}, [], ['a', 'b'], 'foo', True, False, None)  # FIX: missing int & float
TEST_PREFIX_TOKEN_COLOR = '\x1b[04m\x1b[91m'


@pytest.mark.parametrize('data_prefix', TEST_JSON_XXSI_PREFIXES)
@pytest.mark.parametrize('json_data', TEST_JSON_VALUES)
@pytest.mark.parametrize('pretty', PRETTY_MAP.keys())
@responses.activate
def test_json_formatter_with_body_preceded_by_non_json_data(data_prefix, json_data, pretty):
    """Test JSON bodies preceded by non-JSON data."""
    body = data_prefix + json.dumps(json_data)
    content_type = 'application/json'
    responses.add(responses.GET, URL_EXAMPLE, body=body,
                  content_type=content_type)

    colored_output = pretty in ('all', 'colors')
    env = MockEnvironment(colors=256) if colored_output else None
    r = http('--pretty=' + pretty, URL_EXAMPLE, env=env)

    indent = None if pretty in ('none', 'colors') else 4

Review comment (suggestion):
```suggestion
from httpie.cli.constants import PRETTY_MAP
from httpie.compat import is_windows
```

Code after:
import json

import pytest
import responses

from httpie.cli.constants import PRETTY_MAP
from httpie.compat import is_windows
from httpie.output.formatters.colors import ColorFormatter

from .utils import MockEnvironment, http, URL_EXAMPLE

TEST_JSON_XXSI_PREFIXES = (r")]}',\n", ")]}',", 'while(1);', 'for(;;)', ')', ']', '}')
TEST_JSON_VALUES = ({}, {'a': 0, 'b': 0}, [], ['a', 'b'], 'foo', True, False, None)  # FIX: missing int & float
TEST_PREFIX_TOKEN_COLOR = '\x1b[38;5;15m' if is_windows else '\x1b[04m\x1b[91m'


@pytest.mark.parametrize('data_prefix', TEST_JSON_XXSI_PREFIXES)
@pytest.mark.parametrize('json_data', TEST_JSON_VALUES)
@pytest.mark.parametrize('pretty', PRETTY_MAP.keys())
@responses.activate
def test_json_formatter_with_body_preceded_by_non_json_data(data_prefix, json_data, pretty):
    """Test JSON bodies preceded by non-JSON data."""
    body = data_prefix + json.dumps(json_data)
    content_type = 'application/json'
    responses.add(responses.GET, URL_EXAMPLE, body=body,
                  content_type=content_type)

    colored_output = pretty in ('all', 'colors')
    env = MockEnvironment(colors=256) if colored_output else None
    r = http('--pretty=' + pretty, URL_EXAMPLE, env=env)
Repository: httpie/httpie
File: httpie/cli/definition.py (Python)

Code before:

    '''
)
output_processing.add_argument(
    '--sorted',
    **_sorted_kwargs,
    help=f'''
    Re-enables all sorting options while formatting output. It is a shortcut for:

        --format-options={SORTED_FORMAT_OPTIONS_STRING}

    '''
)

output_processing.add_argument(
    '--response-content-type',
    metavar='CONTENT_TYPE',
    help='''
        The response Content-Type can be specified explicitly via this option.
        Useful when the Content-Type returned by the server is incorrect.
        It is a shortcut for:

            --format-options=response.content_type:CONTENT_TYPE

        Example:

            --response-content-type 'application/xml'
    '''
)


output_processing.add_argument(
    '--format-options',
    action='append',
    help='''
    Controls output formatting. Only relevant when formatting is enabled
    through (explicit or implied) --pretty=all or --pretty=format.
    The following are the default options:

        {option_list}

    You may use this option multiple times, as well as specify multiple
    comma-separated options at the same time. For example, this modifies the
    settings to disable the sorting of JSON keys, and sets the indent size to 2:

        --format-options json.sort_keys:false,json.indent:2

    This is something you will typically put into your config file.

    '''.format(
        option_list='\n'.join(

Review comment (suggestion):
In #1110 I will add more examples like:
```shell
--response-content-type 'application/xml; charset=utf-8'
--response-content-type 'charset=utf-8'
```


Code after:

    '''
)
output_processing.add_argument(
    '--sorted',
    **_sorted_kwargs,
    help=f'''
    Re-enables all sorting options while formatting output. It is a shortcut for:

        --format-options={SORTED_FORMAT_OPTIONS_STRING}

    '''
)

output_processing.add_argument(
    '--response-as',
    metavar='CONTENT_TYPE',
    help='''
    Override the response Content-Type for formatting purposes, e.g.:

        --response-as=application/xml

    It is a shortcut for:

        --format-options=response.as:CONTENT_TYPE
    '''
)


output_processing.add_argument(
    '--format-options',
    action='append',
    help='''
    Controls output formatting. Only relevant when formatting is enabled
    through (explicit or implied) --pretty=all or --pretty=format.
    The following are the default options:

        {option_list}

    You may use this option multiple times, as well as specify multiple
    comma-separated options at the same time. For example, this modifies the
    settings to disable the sorting of JSON keys, and sets the indent size to 2:

        --format-options json.sort_keys:false,json.indent:2

    This is something you will typically put into your config file.

    '''.format(
        option_list='\n'.join(
            f'        {option}' for option in DEFAULT_FORMAT_OPTIONS).strip()
    )
Repository: httpie/httpie
File: httpie/cli/definition.py (Python)

Code before:
    **_unsorted_kwargs,
    help=f'''
    Disables all sorting while formatting output. It is a shortcut for:

        --format-options={UNSORTED_FORMAT_OPTIONS_STRING}

    '''
)
output_processing.add_argument(
    '--sorted',
    **_sorted_kwargs,
    help=f'''
    Re-enables all sorting options while formatting output. It is a shortcut for:

        --format-options={SORTED_FORMAT_OPTIONS_STRING}

    '''
)

output_processing.add_argument(
    '--response-content-type',
    metavar='CONTENT_TYPE',
    help='''
    Override the response Content-Type. It will be effective when the response
    is pretty-printed, and can be used when the server returns an incorrect
    Content-Type regarding the contents. It is a shortcut for:

        --format-options=response.content_type:CONTENT_TYPE
    '''
)


output_processing.add_argument(
    '--format-options',
    action='append',
    help='''
    Controls output formatting. Only relevant when formatting is enabled
    through (explicit or implied) --pretty=all or --pretty=format.
    The following are the default options:

        {option_list}

    You may use this option multiple times, as well as specify multiple
    comma-separated options at the same time. For example, this modifies the
    settings to disable the sorting of JSON keys, and sets the indent size to 2:

        --format-options json.sort_keys:false,json.indent:2

    This is something you will typically put into your config file.

    '''.format(

Review comment (suggestion):
```suggestion
    Override the response Content-Type for formatting purposes, e.g.:
    
        --response-content-type=application/xml
    
    It is a shortcut for:
```

Code after:
    **_unsorted_kwargs,
    help=f'''
    Disables all sorting while formatting output. It is a shortcut for:

        --format-options={UNSORTED_FORMAT_OPTIONS_STRING}

    '''
)
output_processing.add_argument(
    '--sorted',
    **_sorted_kwargs,
    help=f'''
    Re-enables all sorting options while formatting output. It is a shortcut for:

        --format-options={SORTED_FORMAT_OPTIONS_STRING}

    '''
)

output_processing.add_argument(
    '--response-as',
    metavar='CONTENT_TYPE',
    help='''
    Override the response Content-Type for formatting purposes, e.g.:

        --response-as=application/xml

    It is a shortcut for:

        --format-options=response.as:CONTENT_TYPE
    '''
)


output_processing.add_argument(
    '--format-options',
    action='append',
    help='''
    Controls output formatting. Only relevant when formatting is enabled
    through (explicit or implied) --pretty=all or --pretty=format.
    The following are the default options:

        {option_list}

    You may use this option multiple times, as well as specify multiple
    comma-separated options at the same time. For example, this modifies the
    settings to disable the sorting of JSON keys, and sets the indent size to 2:

        --format-options json.sort_keys:false,json.indent:2

    This is something you will typically put into your config file.