lpil/yesql

sql file transferred from linux to windows "\r" gets added to keyword with unspecific error

tschnibo opened this issue · 4 comments

Hey,

Than you for your package!

Minor thing here... was just tinkering about and figured that if I have a empty line after the :keyword atom in the .sql file. And then transfer the whole think to windows, I suddenly get an error for ' not givenmissingParam) Required parameter ' :keyword.
Maybe the file was originally created on windows.

I found that the fetch_param function gets the key as :"keyword\r".

I guess one should not have trailing empty lines in the file anyways. But maybe one could still automatically remove the "\r" or give at least a more ideal warning.

Query.testquery([atc: "J02"])
param in fetch_param :"atc\r"
gives
not givenissingParam) Required parameter :atc
So at first I did not see that there was a "\r" parsed into the key.

I am an elixir beginner. And by no means an interoop specialist. Would it be ok to replace:
{:ok, sql, param_spec} = file_path |> File.read!() |> Yesql.parse()
with
{:ok, sql, param_spec} = file_path |> File.read!() |> String.replace("\r\n", "\n") |> Yesql.parse()
or does that have downsides for other operating systems or maybe more complex queries?
For my specific file this helps (beside of course deleting the control character...).

Many thanks and best regards
Tschnibo

lpil commented

Hello! I think that newline replacement you suggested would work OK. Would you mind making a PR?

hey, thank you for the extremely fast response! Honestly never done a pull request. But I'll read up and try 👍 just takes a bit longer than your answer maybe ;) - good opportunity to try I guess ;)

Working on it (getting a postgres container for the tests...) does it make sense to introduce a seperate "test" for this issue?

lpil commented

Thank you!