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.

Sign up for GitHub

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

Jump to bottom

Draft: Run initial JuliaFormatter with BlueStyle #197

Closed
wants to merge 5 commits into from
Closed

Draft: Run initial JuliaFormatter with BlueStyle #197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0e908df
Run initial JuliaFormatter with BlueStyle
nickrobinson251 Sep 22, 2020
3e4c79e
Undo formatting of `error_codes` and `typemaps`
nickrobinson251 Sep 24, 2020
50f873a
Exclude formatting of `error_codes` and `typemaps`
nickrobinson251 Sep 24, 2020
d95ed7b
Re-format with JuliaFormatter v0.9.1
nickrobinson251 Sep 24, 2020
15a3bd2
Run updated JuliaFormatter@0.9.7
nickrobinson251 Sep 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion deps/error_codes.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ using HTTP


const ERROR_CODE_APPENDIX = "https://www.postgresql.org/docs/current/errcodes-appendix.html"
const HEADER = "# autogenerated by deps/error_codes.jl using $ERROR_CODE_APPENDIX"
const HEADER = """
# autogenerated by deps/error_codes.jl using $ERROR_CODE_APPENDIX
#! format: off"""

const SUCCESS_CLASS = "C00"
const WARNING_CLASSES = ("C01", "C02")
Expand Down
48 changes: 27 additions & 21 deletions src/LibPQ.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
module LibPQ

export status, reset!, execute, prepare, async_execute, cancel,
num_columns, num_rows, num_params, num_affected_rows

export status,
reset!,
execute,
prepare,
async_execute,
cancel,
num_columns,
num_rows,
num_params,
num_affected_rows

using Base: Semaphore, acquire, release
using Base.Iterators: zip, product
Expand All @@ -21,42 +28,41 @@ using Memento: Memento, getlogger, warn, info, error, debug
using OffsetArrays
using TimeZones

const Parameter = Union{String, Missing}
const Parameter = Union{String,Missing}
const LOGGER = getlogger(@__MODULE__)

function __init__()
INTERVAL_REGEX[] = _interval_regex()
Memento.register(LOGGER)
return Memento.register(LOGGER)
nickrobinson251 marked this conversation as resolved.
Show resolved Hide resolved
end

# Docstring template for types using DocStringExtensions
@template TYPES =
"""
$(TYPEDEF)
@template TYPES = """
$(TYPEDEF)

$(DOCSTRING)
$(DOCSTRING)

## Fields:
## Fields:

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

@nickrobinson251 nickrobinson251 Sep 22, 2020 •
edited
Loading

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.

yes please!
JuliaDiff/BlueStyle#60

Copy link
Author

Choose a reason for hiding this comment

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

JuliaDiff/BlueStyle#71

Copy link
Author

@nickrobinson251 nickrobinson251 Sep 28, 2020 •
edited
Loading

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)


include(joinpath(@__DIR__, "utils.jl"))

module libpq_c
export Oid
export Oid
iamed2 marked this conversation as resolved.
Show resolved Hide resolved

@static if VERSION < v"1.3.0"
include(joinpath(@__DIR__, "..", "deps", "deps.jl"))
@static if VERSION < v"1.3.0"
include(joinpath(@__DIR__, "..", "deps", "deps.jl"))

function __init__()
check_deps()
end
else
using LibPQ_jll
function __init__()
return check_deps()
end
else
using LibPQ_jll
end

include(joinpath(@__DIR__, "headers", "libpq-fe.jl"))
include(joinpath(@__DIR__, "headers", "libpq-fe.jl"))
end

using .libpq_c
Expand Down
40 changes: 19 additions & 21 deletions src/asyncresults.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ function Base.show(io::IO, async_result::AsyncResult)
else
"in progress"
end
print(io, typeof(async_result), " (", status, ")")
return print(io, typeof(async_result), " (", status, ")")
iamed2 marked this conversation as resolved.
Show resolved Hide resolved
end

"""
Expand All @@ -56,11 +56,7 @@ function handle_result(async_result::AsyncResult; throw_error=true)
for result_ptr in _consume(async_result.jl_conn)
try
result = handle_result(
Result(
result_ptr,
async_result.jl_conn;
async_result.result_kwargs[]...
);
Result(result_ptr, async_result.jl_conn; async_result.result_kwargs[]...);
throw_error=throw_error,
)
catch err
Expand Down Expand Up @@ -102,9 +98,11 @@ function _consume(jl_conn::Connection)
_cancel(jl_conn)
end

last_log == curr && debug(LOGGER, "Waiting to read from connection $(jl_conn.conn)")
last_log == curr &&
debug(LOGGER, "Waiting to read from connection $(jl_conn.conn)")
wait(watcher)
last_log == curr && debug(LOGGER, "Consuming input from connection $(jl_conn.conn)")
last_log == curr &&
debug(LOGGER, "Consuming input from connection $(jl_conn.conn)")
iamed2 marked this conversation as resolved.
Show resolved Hide resolved
success = libpq_c.PQconsumeInput(jl_conn.conn) == 1
!success && error(LOGGER, Errors.PQConnectionError(jl_conn))

Expand All @@ -116,8 +114,8 @@ function _consume(jl_conn::Connection)
return result_ptrs
else
result_num = length(result_ptrs) + 1
debug(LOGGER,
"Saving result $result_num from connection $(jl_conn.conn)"
debug(
LOGGER, "Saving result $result_num from connection $(jl_conn.conn)"
)
push!(result_ptrs, result_ptr)
end
Expand All @@ -126,9 +124,10 @@ function _consume(jl_conn::Connection)
catch err
if err isa Base.IOError && err.code == -9 # EBADF
debug(() -> sprint(showerror, err), LOGGER)
error(LOGGER, Errors.JLConnectionError(
"PostgreSQL connection socket was unexpectedly closed"
))
error(
LOGGER,
Errors.JLConnectionError("PostgreSQL connection socket was unexpectedly closed"),
)
else
rethrow(err)
end
Expand Down Expand Up @@ -158,9 +157,10 @@ function _cancel(jl_conn::Connection)
errbuf = zeros(UInt8, errbuf_size)
success = libpq_c.PQcancel(cancel_ptr, pointer(errbuf), errbuf_size) == 1
if !success
warn(LOGGER, Errors.JLConnectionError(
"Failed cancelling query: $(String(errbuf))"
))
warn(
LOGGER,
nickrobinson251 marked this conversation as resolved.
Show resolved Hide resolved
Errors.JLConnectionError("Failed cancelling query: $(String(errbuf))"),
)
else
debug(LOGGER, "Cancelled query for connection $(jl_conn.conn)")
end
Expand Down Expand Up @@ -218,8 +218,8 @@ end
function async_execute(
jl_conn::Connection,
query::AbstractString,
parameters::Union{AbstractVector, Tuple};
kwargs...
parameters::Union{AbstractVector,Tuple};
kwargs...,
)
string_params = string_parameters(parameters)
pointer_params = parameter_pointers(string_params)
Expand Down Expand Up @@ -260,9 +260,7 @@ function _async_submit(conn_ptr::Ptr{libpq_c.PGconn}, query::AbstractString)
end

function _async_submit(
conn_ptr::Ptr{libpq_c.PGconn},
query::AbstractString,
parameters::Vector{Ptr{UInt8}},
conn_ptr::Ptr{libpq_c.PGconn}, query::AbstractString, parameters::Vector{Ptr{UInt8}}
)
num_params = length(parameters)

Expand Down
Loading