Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[#224] Add description for multiline puzzle #237
[#224] Add description for multiline puzzle #237
Changes from 1 commit
2f83a39
8e26e6f
a2acecf
0c10776
194a4ba
a71e657
291030b
bb3f370
438d324
4e4fe39
53b9be5
317b976
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why it was removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not supported in the code, there is only a logic to remove prefix, but no logic to remove any kinds of symbols from the end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, I'd suggest to fix/improve support for such type of comments instead of removing it from documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a bug about it #225
I think it is better to remove it now and return it back after fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pnatashap If we remove this examples, I believe we have to remove
TEST-13
and42
from the next sentence too:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same: why it has been removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not supported in the code, there is only a logic to remove prefix, but no logic to remove any kinds of symbols from the end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pnatashap What Is "prefix" here?
~
? I haven't found the definition of "prefix" in theREADME
file.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can be used to decorate text, not required, but for there is no restrictions about the prefix, it should be just equal for all text. So it is just an example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pnatashap, maybe we can add this to the documentation? I mean, we can state that the "prefix" is a symbol that can be used to decorate text; it isn't required. And provide some examples.
Moreover, I would suggest adding some headers or small - one-sentence descriptions to these code examples:
\
symbol to connect multiline puzzles."What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO we should reconsider and improve this rule. Using a trailing space for making decisions isn't good.
First, it's non-obvious.
Second, it's invisible, so it's really hard to catch such issues on code review.
Third, users may want to run auto-formatting tools, that can remove such "noise". In this case, users will face a massive close and creation of the issues.
How about using a backslash? Like this:
@yegor256 WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pnatashap @php-coder indeed, a "blind" space at the end of a line is provocation of a mistake
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good symbol should be clear enough for developers.
Also will check if it works for second line as described in #163