-
-
Notifications
You must be signed in to change notification settings - Fork 338
fix: Trim triple newlines instead of double newlines #2434
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
Conversation
| end | ||
|
|
||
| @doc false | ||
| def trim_double_newlines(str) do |
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.
We can't remove this right now because AshPhoenix is using it. Also I see a formatting error, is that from these changes or in main? Either way, will merge once this is added back in 😄
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 got to the same realization when I went to sleep last night, sorry about that!
The formatting changes are caused by my changes, though there was also a formatting error in actions/destroy/bulk.ex which I've fixed in a separate commit.
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.
See also my comment in ash-project/ash_phoenix#439 (comment)
|
Huh, the formatter in the CI behaves differently from my local install, with regards to that bulk destroy change. I'll remove that commit again. |
Removing double newlines breaks formatting in e.g. the generated browser docs, and breaks doctests, since they expect the example code sections to be separated by the rest of the comments by double newlines. Instead, we should focus on collapsing triple newlines, which can occur for legitimate reasons, e.g. a section of the generated documentation doesn't return any text, but which shouldn't be showed in the resulting documentation. Fixes: ash-project#2433
|
Thanks for pointing out the issues here! I'm just going to update the |
|
This will then automatically resolve the issue in |
|
Thanks! |
Removing double newlines breaks formatting in e.g. the generated browser docs,
and breaks doctests, since they expect the example code sections to be separated
by the rest of the comments by double newlines.
Instead, we should focus on collapsing triple newlines, which can occur for
legitimate reasons, e.g. a section of the generated documentation doesn't return
any text, but which shouldn't be showed in the resulting documentation.
Fixes: #2433
I tried adding a regression test to
code_interface_test.exsby adding the following, but it didn't work becauseCode.fetch_docsreturned{:error, :module_not_found}:Any suggestions for a better way to test this would be greatly appreciated.
Contributor checklist
Leave anything that you believe does not apply unchecked.
Edit: Updated to reflect this comment: #2433 (comment)