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

Use witdh 1 for special chars when slipping strings #424

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

Conversation

madchicken
Copy link

A naive attempt to fix issue #410

The PR adds 1 to the width of a string when a special char is encountered.

@madchicken
Copy link
Author

Sorry, I had to reopen this PR due to a bad command with the previous one.

@zhiburt
Copy link
Owner

zhiburt commented Aug 30, 2024

Getting back to this;

I am sure,
We do must not panic/break things in case someone did not prepared his strings in advance.

But I am not sure whether it's the right fix.
Maybe it is.
But somehow I doubt.

If it would be free but this if is actually may be costly enough from performance stand point of view.

I guess this issue must be solved on the architecture level --- somehow --- 🤔 💭

Just left some thoughts here.

Have a great weekend and appreciate your work.
Let me know what you think.

@zhiburt
Copy link
Owner

zhiburt commented Aug 30, 2024

Maybe like having a DIRTY marker to indicate that we shall do some processing beforehand.
And it would be more appropriate to do that right away I guess.

@zhiburt zhiburt added the enhancement New feature or request label Aug 30, 2024
@zhiburt
Copy link
Owner

zhiburt commented Aug 30, 2024

But tabled::settings::formatting::Charset was introduced just for that....
Maybe your approach makes scenes, but having a flag to turn it off?

I mean it's all in regard that it'd be ideal to not pay for what is not used in most cases.

Just thoughts outloud

@madchicken
Copy link
Author

@zhiburt, I totally agree that this is probably a very dirty approach to solving this issue. I opened this PR more to raise the problem than proposing a real solution. On the other side, knowing if a header contains or not a break line or a TAB char can be difficult, especially if the value is loaded at runtime. Having a flag can be a solution but I feel it is dirtier than the fix itself 😄
A question for you: do you feel that not counting those characters in the with of a string is correct? Since it seems to me that the underlying function takes a very strong assumption by not doing it...

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

Successfully merging this pull request may close these issues.

2 participants