Skip to content
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

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Paalon
Copy link
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
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
Collaborator

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
Collaborator

@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 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
Collaborator

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
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.

@@ -80,11 +99,9 @@ done(it::YAMLDocIterator, state) = it.next_doc === nothing
iterate(it::YAMLDocIterator) = next(it, start(it))
iterate(it::YAMLDocIterator, s) = done(it, s) ? nothing : next(it, s)

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

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