Skip to content

[Not Urgent, Stylistic] Format Julia source files by JuliaFormatter#157

Open
Paalon wants to merge 2 commits intoJuliaData:masterfrom
Paalon:formatting
Open

[Not Urgent, Stylistic] Format Julia source files by JuliaFormatter#157
Paalon wants to merge 2 commits intoJuliaData:masterfrom
Paalon:formatting

Conversation

@Paalon
Copy link
Copy Markdown
Contributor

@Paalon Paalon commented Jun 11, 2024

Format Julia source files by JuliaFormatter. Everyone can format sources by executing the following in the project root directory:

using JuliaFormatter

format(".")

Formatting styles are set in .JuliaFormatter.toml. If there are problems or preferences, tell me.

This solves #154.

@Paalon
Copy link
Copy Markdown
Contributor Author

Paalon commented Jun 11, 2024

This PR will conflict with #153, #155 and #156, therefore if you merge this, you should merge #153, #155 and #156 first, then I reformat sources and I resolve the confliction. Finally, merge this (#157).

@kescobo
Copy link
Copy Markdown
Member

kescobo commented Jun 11, 2024

Does it make sense to (also?) make a precommit hook (I think that's what it's called) so this stays consistent?

Copy link
Copy Markdown
Member

@kescobo kescobo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm - some of these are definitely appropriate, but I'm not in love with all of them. Then again, I'm not routinely looking at this source code, so it it makes sense for you, I'm ok with it

Comment thread src/YAML.jl
Comment on lines +40 to +48
load(
ts::TokenStream,
more_constructors::_constructor=nothing,
multi_constructors::Dict=Dict();
dicttype::_dicttype=Dict{Any, Any},
constructorType::Function=SafeConstructor,
) = load(
ts,
constructorType(_patch_constructors(more_constructors, dicttype), multi_constructors),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love this formatting

Copy link
Copy Markdown
Contributor

@GunnarFarneback GunnarFarneback Jun 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. This is the kind of JuliaFormatter thing that makes me not want to touch a code base.

This particular definition shouldn't have been in short form at all though; there's way too many arguments to make that look good, regardless of formatting. Never mind, Juliaformatter makes long form definitions equally horrible.

Comment thread src/YAML.jl

load_all(input::IO, args...; kwargs...) =
YAMLDocIterator(input, args...; kwargs...)
load_all(input::IO, args...; kwargs...) = YAMLDocIterator(input, args...; kwargs...)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This I agree with

@Paalon Paalon changed the title Format Julia source files by JuliaFormatter [Not Urgent, Stylistic] Format Julia source files by JuliaFormatter Jun 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants