-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 first doc comment inside macro yield #15050
Fix first doc comment inside macro yield #15050
Conversation
@@ -104,7 +104,7 @@ module Crystal | |||
if (loc = @last.location) && loc.filename.is_a?(String) || is_yield | |||
macro_expansion_pragmas = @macro_expansion_pragmas ||= {} of Int32 => Array(Lexer::LocPragma) | |||
(macro_expansion_pragmas[@str.pos.to_i32] ||= [] of Lexer::LocPragma) << Lexer::LocPushPragma.new | |||
@str << "begin " if is_yield | |||
@str << "begin\n" if is_yield | |||
@last.to_s(@str, macro_expansion_pragmas: macro_expansion_pragmas, emit_doc: true) | |||
@str << " end" if is_yield |
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.
Perhaps this should be changed accordingly?
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.
I don't think there's a situation where this could cause harm.
I added a spec for this using the example from the OP. |
Co-authored-by: Julien Portalier <[email protected]>
The following example eats the doc comment:
This is because the first line of comment is generated on the same line as the
begin
which is inserted when using{{yield}}
, like so:Using a newline instead of whitespace after
begin
in macro yield fixes this.