-
Notifications
You must be signed in to change notification settings - Fork 41
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
Fix snippet reader tab and indent handling #161
base: master
Are you sure you want to change the base?
Conversation
I don't understand this change. It does not retain the original format, what does it address? |
It fixes the indentation behaviour when the source contains tabs. Without the fix the indentation is not correctly handled. Without the fix the source code block has too many tabs and is not rendered in a nice format. |
But looking at your examples it does remove tabs as well, no? |
As far as I recall - no - tabs are not removed at all. See first example. |
First snippet has two tabs, but expected does not have any. What do I miss? |
The tabs from this example |
Why? If the snippet is intended that way? How can we know what is right or wrong in snippet content and context? |
@kwin, do you understand this PR? |
The snippet renderer removes unnecessary indentation. It thus moves the code block left. A code example which is highly indented will thus not include the bogus indentation. {
{
snipped-start
{
// your code
}
snipped-end
}
} should result in {
// your code
} and not {
// your code
} |
But what if
is correct indentation? |
The code clearly removes this indentation if it is done by using spaces. Using tabs it is not removed and thus looks broken. |
It is hard to say what is right, but I would say that a snippet should remain as-is and not modified, as ugly as it is. |
I think the indentation is clearly removed in order to prevent horizontal scrollbars for included snippets. |
I'd prefer to leave formatting as-is, but I am open to a snippet option which reverse-indents. |
I'd like to perform a release this weekend. Anyone want to complete the discussion with me? |
The PR addresses the following issues:
Snippet Source:
Old Behaviour
New Behaviour:
Snippet Source:
Old Behaviour
New Behaviour: