Skip to content

Add binary_display configuration option#1512

Open
rolandwalker wants to merge 1 commit intomainfrom
RW/add-binary-as-text-config-option
Open

Add binary_display configuration option#1512
rolandwalker wants to merge 1 commit intomainfrom
RW/add-binary-as-text-config-option

Conversation

@rolandwalker
Copy link
Contributor

@rolandwalker rolandwalker commented Feb 5, 2026

Description

This is the rough equivalent of the vendor --skip-binary-as-text CLI option, which has the effect of reversing

when set to "utf8".

I'm still not sure how to write tests for non-default myclirc options, though there may have been a recent example. Tests incorporated thanks to @scottnemes .

Fixes #1511.

Checklist

  • I added this contribution to the changelog.md file.
  • I added my name to the AUTHORS file (or it's already there).
  • To lint and format the code, I ran
    uv run ruff check && uv run ruff format && uv run mypy --install-types .

@rolandwalker rolandwalker force-pushed the RW/add-binary-as-text-config-option branch from e0676ab to fd133c2 Compare February 5, 2026 09:36
@rolandwalker rolandwalker changed the title Add binary_as_text configuration option Add binary_as_text configuration option Feb 5, 2026
@rolandwalker rolandwalker self-assigned this Feb 5, 2026
@rolandwalker rolandwalker force-pushed the RW/add-binary-as-text-config-option branch from fd133c2 to 8dde329 Compare February 5, 2026 09:57
@rolandwalker
Copy link
Contributor Author

rolandwalker commented Feb 5, 2026

Maybe the configuration option should be non-boolean, like

binary_display = hex

Edit: updated the PR to be a non-boolean binary_display as above. This way there could be more choices in the future.

@rolandwalker rolandwalker force-pushed the RW/add-binary-as-text-config-option branch from 8dde329 to f4fe363 Compare February 5, 2026 20:43
@rolandwalker rolandwalker changed the title Add binary_as_text configuration option Add binary_display configuration option Feb 5, 2026
@scottnemes
Copy link
Contributor

Got some tests that should work:

@dbtest
def test_binary_display_hex(executor, capsys):
    m = MyCli()
    m.sqlexecute = SQLExecute(
        None,
        USER,
        PASSWORD,
        HOST,
        PORT,
        None,
        None,
        None,
        None,
        None,
        None,
        None,
        None,
        None,
        None,
    )
    m.explicit_pager = False
    sqlresult = next(m.sqlexecute.run("select b'01101010' AS binary_test"))
    formatted = m.format_output(
        sqlresult.title,
        sqlresult.results,
        sqlresult.headers,
        False,
        False,
        "<nope>",
        "right",
        "hex",
        None,
    )
    m.output(formatted, sqlresult.status)
    expected = "+-------------+\n| binary_test |\n+-------------+\n| 0x6a        |\n+-------------+\n1 row in set\n"
    stdout = capsys.readouterr().out
    assert expected in stdout


@dbtest
def test_binary_display_utf8(executor, capsys):
    m = MyCli()
    m.sqlexecute = SQLExecute(
        None,
        USER,
        PASSWORD,
        HOST,
        PORT,
        None,
        None,
        None,
        None,
        None,
        None,
        None,
        None,
        None,
        None,
    )
    m.explicit_pager = False
    sqlresult = next(m.sqlexecute.run("select b'01101010' AS binary_test"))
    formatted = m.format_output(
        sqlresult.title,
        sqlresult.results,
        sqlresult.headers,
        False,
        False,
        "<nope>",
        "right",
        "utf8",
        None,
    )
    m.output(formatted, sqlresult.status)
    expected = "+-------------+\n| binary_test |\n+-------------+\n| j           |\n+-------------+\n1 row in set\n"
    stdout = capsys.readouterr().out
    assert expected in stdout

@rolandwalker rolandwalker force-pushed the RW/add-binary-as-text-config-option branch 7 times, most recently from 7c6b7f8 to 6b9f630 Compare February 6, 2026 09:52
This is the rough equivalent of the vendor --skip-binary-as-text CLI
option, which has the effect of reversing

 * #1483

when set to "utf8".
@rolandwalker rolandwalker force-pushed the RW/add-binary-as-text-config-option branch from 6b9f630 to 2cec689 Compare February 6, 2026 10:04
@rolandwalker
Copy link
Contributor Author

When I incorporate the tests, the new tests pass, but then the behave suite fails

and only in CI, not on my local.

@scottnemes
Copy link
Contributor

When I incorporate the tests, the new tests pass, but then the behave suite fails

and only in CI, not on my local.

Yeah the whole test suite works fine for me as well. Weird! I'll look at it more later to see if I can find anything. No idea how it would affect other tests though, especially the entirely different test suite.

@scottnemes
Copy link
Contributor

scottnemes commented Feb 7, 2026

When I incorporate the tests, the new tests pass, but then the behave suite fails

and only in CI, not on my local.

Testing this out on another PR, and it is working. I based my branch off of main, so guessing there is some combo of other stuff your branch has (or doesn't have). See my PR for the diffs:
#1514

I did notice that the mycli/myclirc has the option null_string=<null> while the test/myclirc has null_string=<nope>. The test is looking for the <nope> value, but that was working previously. So guessing there is something to do with recent config changes that there is some combo of changes where it is now using the null_string value from mycli/myclirc instead of the test one perhaps.

Edit:

Don't think I had your latest changes on my last test, so I did it again (less diffs this time), but tests all passed again. So seems something is/was going on when you pushed the tests the first time.

#1515

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.

[possible degradation] UTF-8 string is now printed in 0x hex form in some cases.

2 participants