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

Make string difference more readable #89

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

Conversation

haobug
Copy link
Contributor

@haobug haobug commented Jun 24, 2018

With this patch:

    assertEquals "deadbeef" "deadbuff"

will output like this

    ASSERT:
    expected:<deadbeef>
    ---------------^
    but was :<deadbuff>

@williamdes williamdes requested a review from kward October 24, 2021 18:36
@kward
Copy link
Owner

kward commented Oct 26, 2021

I agree that this output is cleaner, but this commit will change the behavior for all users, and I am hesitant to make such a change due to the wide userbase.

Alternatively, this could be controlled with a flag (variable) so that users can choose to enable it, similar to the SHUNIT_COLOR flag. Maybe call it SHUNIT_DENSITY which relates to the density of the output? Values could be 'compact' (the default) or 'comfortable' (yours).

Thoughts?

@kward
Copy link
Owner

kward commented Oct 26, 2021

The pull as is needs to be rewritten to align with coding standards (function name, variable names, variable cleanup, etc.).
https://github.com/kward/shunit2/wiki/Coding-standards

@haobug
Copy link
Contributor Author

haobug commented Nov 1, 2021

I agree that this output is cleaner, but this commit will change the behavior for all users, and I am hesitant to make such a change due to the wide userbase.

yes, I have realized that. it might break someone's code.

Alternatively, this could be controlled with a flag (variable) so that users can choose to enable it, similar to the SHUNIT_COLOR flag. Maybe call it SHUNIT_DENSITY which relates to the density of the output? Values could be 'compact' (the default) or 'comfortable' (yours).

Thoughts?

@kward could you give out some reference commit that can show me how to add a new flag compatibility?

@haobug
Copy link
Contributor Author

haobug commented Nov 1, 2021

The pull as is needs to be rewritten to align with coding standards (function name, variable names, variable cleanup, etc.). https://github.com/kward/shunit2/wiki/Coding-standards

I will revise the lines of modified code to align with the coding style in the commit of changing flag controlled way

With this patch:
```
    assertEquals "deadbeef" "deadbuff"
```
will output like this
```
    ASSERT:
    expected:<deadbeef>
    ---------------^
    but was :<deadbuff>
```

set env variable SHUNIT_DIFF_ALIGN=[always/never/auto]
@haobug haobug force-pushed the more_readable_diff branch from 58620f6 to a47a266 Compare March 12, 2023 14:31
@haobug
Copy link
Contributor Author

haobug commented Mar 12, 2023

@kward i have finished my code change revising of both styling and adding a SHUNIT_DIFF_ALIGN env variable.
the default of SHUNIT_DIFF_ALIGN is never. one can use it as following

SHUNIT_DIFF_ALIGN=always bash ut.sh

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.

3 participants