Add option to configure non-standard trailing newlines#208
Conversation
| # Add or remove trailing newlines depending on global config | ||
| # The default assumption is one trailing newline | ||
| newlines = haskey(global_cfg["files"], file) ? get(global_cfg["files"][file], "trailing_newlines", 1) : 1 | ||
| write(io, "\n" ^ newlines) |
There was a problem hiding this comment.
Since ^ is a bit unusual for strings in most programming languages, here's an example:
julia> "x" ^ 5
"xxxxx"
|
This might cause unwanted changes if people added leading newlines to their CoC append, see exercism/julia#500 (fixed by exercism/julia@9d25dbd) |
| # Add or remove trailing newlines depending on global config | ||
| # The default assumption is one trailing newline | ||
| newlines = haskey(global_cfg["files"], file) ? get(global_cfg["files"][file], "trailing_newlines", 1) : 1 | ||
| write(io, "\n" ^ newlines) |
| # File-specific config | ||
| [files] | ||
| [files."CODE_OF_CONDUCT.md"] | ||
| trailing_newlines = 2 |
There was a problem hiding this comment.
This was there just to prove it was working, right? https://github.com/exercism/julia/pull/500/files
There was a problem hiding this comment.
Kind of. I think it's sensible that any append to that file is separated by two newlines as a new paragraph.
ErikSchierboom
left a comment
ErikSchierboom
left a comment
There was a problem hiding this comment.
LGTM! Thanks
@ee7 Once this is merged, could you rebase your PR on main? That way we can do a test run with this functionality included in your PR
Sure. So the idea is that we alter #205 to read from |
No, it should work differently. Every file ends with exactly 1 newline within the repo, but when the org-wide-files syncing workflow runs, it'll ensure that the number of newlines specified in the |
|
Oh, I see. That sounds better. |
ee7
left a comment
ee7
left a comment
There was a problem hiding this comment.
I'll approve, assuming that it works as Erik says it does :)
| write(io, "\n" ^ newlines) | ||
|
|
||
| # Write append content | ||
| write(io, append_content) |
There was a problem hiding this comment.
So the responsibility is still on the downstream repo to end their append file with exactly one newline, so that the combined file also ends with exactly 1 newline, right? That's fine by me.
We could choose to write the combined file with exactly 1 final newline, regardless of the number of newlines in the repo's append file. I don't think it's worth bothering (but feel free to do that if you like it).
There was a problem hiding this comment.
So the responsibility is still on the downstream repo to end their append file with exactly one newline, so that the combined file also ends with exactly 1 newline, right? That's fine by me.
Yes. If their append file does not end with exactly one newline, they either don't care or the append file itself would cause them issues already. I don't think we have to be concerned with those two cases.
|
I've just done a test run and it looks like the newline appending works! https://github.com/exercism/csharp/pull/1921/files |
3 participants
Test run: https://github.com/exercism/org-wide-files/actions/runs/2264221458
cc @ee7