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

Draft: Run initial JuliaFormatter with BlueStyle #197

Closed
wants to merge 5 commits into from

Conversation

nickrobinson251
Copy link

@nickrobinson251 nickrobinson251 commented Sep 22, 2020

julia> format(
                  "src";
                  style=BlueStyle(),
                  always_use_return=true,
                  short_to_long_function_def=true,
                  whitespace_ops_in_indices=true,
                  remove_extra_newlines=true,
                  always_for_in=true,
                  import_to_using=true,
                  pipe_to_function_call=true,
                  whitespace_in_kwargs=false,
                  whitespace_typedefs=true,
             )

EDIT Now re-run with format("src", BlueStyle()) using JuliaFormatter v0.9.1

$(TYPEDFIELDS)
"""
$(TYPEDFIELDS)
"""
Copy link
Author

@nickrobinson251 nickrobinson251 Sep 22, 2020

Choose a reason for hiding this comment

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

This seems like the only particularly odd/unfortunate change

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is something we should add to the style guide (to prefer what previously existed).

The placement of the rest of the text and the closing triple quote relative to the opening triple quote makes no difference to the content of the string.

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Author

@nickrobinson251 nickrobinson251 Sep 28, 2020

Choose a reason for hiding this comment

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

Interestingly, the formatter seems to have been confused here by where the opening triple quotes

By BlueStyle, this should be

@template TYPES = """
        (TYPEDEF)

    (DOCSTRING)

    ## Fields:
    (TYPEDFIELDS)
    """

And JuliaFormatter (v0.9) would leave that as is

julia> str = """
       @template TYPES = \"\"\"
               (TYPEDEF)

           (DOCSTRING)

           ## Fields:
           (TYPEDFIELDS)
           \"\"\"
           """;

julia> println(format_text(str, BlueStyle()))
@template TYPES = """
        (TYPEDEF)

    (DOCSTRING)

    ## Fields:
    (TYPEDFIELDS)
    """

But the actual code here right now has the opening triple-quot on the line below i.e. it is

@template TYPES =
    """
        (TYPEDEF)

    (DOCSTRING)

    ## Fields:
    (TYPEDFIELDS)
    """

which seems to confuse the formatter (?), which changes this to

julia> str = """
       @template TYPES =
           \"\"\"
               (TYPEDEF)

           (DOCSTRING)

           ## Fields:
           (TYPEDFIELDS)
           \"\"\"
           """;

julia> println(format_text(str, BlueStyle()))
@template TYPES = """
                      (TYPEDEF)

                  (DOCSTRING)

                  ## Fields:
                  (TYPEDFIELDS)
                  """

Perhaps @domluna knows if this is intended behaviour (which just needs overloading for BlueStyle) or if something funky is going on?

Copy link

Choose a reason for hiding this comment

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

this is intended, the idea being that the indentation of the string changes based on where the tripe quote opener is moved. This allows it to be moved around intact.

In this case (TYPEDEF) is 4 spaces in behind the opening quote so when the quote is moved up a line, (TYPEDEF) follows to be 4 spaces behind the new position.

Copy link
Author

Choose a reason for hiding this comment

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

okay, cool - thanks - makes sense! BlueStyle prefers something different, so in time it'd be nice to support that for BlueStyle() domluna/JuliaFormatter.jl#283 (comment)

src/connections.jl Outdated Show resolved Hide resolved
src/LibPQ.jl Show resolved Hide resolved
src/LibPQ.jl Show resolved Hide resolved
src/asyncresults.jl Outdated Show resolved Hide resolved
src/asyncresults.jl Outdated Show resolved Hide resolved
src/connections.jl Outdated Show resolved Hide resolved
src/error_codes.jl Outdated Show resolved Hide resolved
src/parsing.jl Outdated Show resolved Hide resolved
src/parsing.jl Show resolved Hide resolved
src/results.jl Outdated Show resolved Hide resolved
src/typemaps.jl Outdated Show resolved Hide resolved
libpq_c.CONNECTION_SETENV => "Negotiating environment-driven parameter settings",
libpq_c.CONNECTION_SSL_STARTUP => "Negotiating SSL encryption",
libpq_c.CONNECTION_CHECK_WRITABLE => "Checking if connection is able to handle write transactions",
libpq_c.CONNECTION_CHECK_WRITABLE =>
"Checking if connection is able to handle write transactions",
Copy link
Author

Choose a reason for hiding this comment

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

i wonder if this case further motivates JuliaDiff/BlueStyle#59 ?

@nickrobinson251
Copy link
Author

Updated formatting with JuliaFormatter v0.9.7.

There are a few open style questions:

Aside from those, I think the only formatter-specific "issue" is the wrong indenting of the multi-line string literal (https://github.com/invenia/LibPQ.jl/pull/197/files#r496234944)

How do you think the format changes in this PR are looking, @iamed2?

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