fix: escape " in check constraint generator

[frankdugan3]

Contributor checklist

Leave anything that you believe does not apply unchecked.

• Bug fixes include regression tests
• Chores
• Documentation changes
• Features include unit/acceptance tests
• Refactoring
• Update dependencies

[frankdugan3]

Actually, I think there's still some problems. Let me investigate further before you check on this.

[frankdugan3]

OK, it's good now, derp! 🤪

[zachdaniel]

Actually, we have to handle already escaped double quotes only right? otherwise this effectively unescapes them, turning \" into \\". So we ned to replace all unescaped double quotes i.e " that start w/ zero or an od number of \.

[frankdugan3]

This is one of those things that makes my brain a little dizzy, lol. I want to get the test case right so I can get the implementation correct.

Given:

check: ~S[title ~= '("\"\\"\\\"\\\\"\\\\\")']

If we replace just odd slashes, we should expect the migration file to have:

check: "title ~= '(\"\\"\\"\\\\"\\\\"\\\\\\")'"

Is that right? 😵‍💫

[frankdugan3]

Thinking about it more, the issue is distinguishing quotes from plain slashes?

So a better check would be:

check: "title ~= '(\"\\\"\\\\\")'"

Becomes:

"check: \"title ~= '(\\\"\\\\\"\\\\\\\")'\""

[zachdaniel]

Thinking about this more, I don't think we need to "fix" this? If you want double quotes in that code, you just need to do \\". closing for now, but feel free to discuss further if you disagree.

[frankdugan3]

Well, it's up to you but I do somewhat strongly disagree because it's inconsistent syntax compared to custom statements and fragments. And if you use multiline sigils, it's very strange, and if using something like ~SQL, it breaks syntax highlighting. It's kind of unexpected that the generated value of the check string differs from that in the resource.

I agree what I did is an ugly solution, though.

Custom statements do this, which bypasses the problem entirely:

def up(%{statement: %{up: up, code?: false}}) do
  """
  execute(\"\"\"
  #{String.trim(up)}
  \"\"\")
  """
end

Perhaps a simpler refactor to do this instead would be a better middle ground like this:

"""
create constraint(:#{as_atom(table)}, :#{as_atom(name)}, check: \"\"\"
#{String.trim(check)}#{option(:prefix, schema)}
\"\"\")
"""

That would be consistent with the other generators, and sidestep my original mess.

[zachdaniel]

I'm down with that solution.