Code folding for PostgreSQL doesn't work as expected.

v6
1

Hi,
I’m using foldGutter @codemirror/language with PostgreSQL dialect. I find the code folding incorrect.
Am I missing any param?
Here is a complex PGSQL code - https://codemirror.net/try/?c=aW1wb3J0IHtiYXNpY1NldHVwLCBFZGl0b3JWaWV3fSBmcm9tICJjb2RlbWlycm9yIgppbXBvcnQgeyBzcWwsIFBvc3RncmVTUUwgfSBmcm9tICdAY29kZW1pcnJvci9sYW5nLXNxbCc7CgpuZXcgRWRpdG9yVmlldyh7CiAgZG9jOiBgLS0gRlVOQ1RJT046IEZlZWRzLlRyYW5zYWN0aW9uRGF0YUFjY291bnROdW1iZXIoYmlnaW50LCBiaWdpbnQsIGludGVnZXIsIGNoYXJhY3RlciB2YXJ5aW5nLCBpbnRlZ2VyLCBjaGFyYWN0ZXIgdmFyeWluZywgaW50ZWdlciwgaW50ZWdlciwgaW50ZWdlciwgYm9vbGVhbiwgY2hhcmFjdGVyIHZhcnlpbmcpXG5cbi0tICBEUk9QIEZVTkNUSU9OIElGIEVYSVNUUyAiRmVlZHMiLiJUcmFuc2FjdGlvbkRhdGFBY2NvdW50TnVtYmVyIihiaWdpbnQsIGJpZ2ludCwgaW50ZWdlciwgY2hhcmFjdGVyIHZhcnlpbmcsIGludGVnZXIsIGNoYXJhY3RlciB2YXJ5aW5nLCBpbnRlZ2VyLCBpbnRlZ2VyLCBpbnRlZ2VyLCBib29sZWFuLCBjaGFyYWN0ZXIgdmFyeWluZyk7XG5cbkNSRUFURSBPUiBSRVBMQUNFIEZVTkNUSU9OICJGZWVkcyIuIlRyYW5zYWN0aW9uRGF0YUFjY291bnROdW1iZXIiKFxuXHQiSW5VcGxvYWRGaWxlUmVnaXN0cmF0aW9uSWQiIGJpZ2ludCxcblx0IkluQWNjb3VudFJlZ2lzdHJhdGlvbklkIiBiaWdpbnQsXG5cdCJJbkFjY291bnROdW1iZXJDb2x1bW5JbmRleCIgaW50ZWdlcixcblx0IkluQWNjb3VudE51bWJlckhlYWRlclZhbHVlIiBjaGFyYWN0ZXIgdmFyeWluZyxcblx0IkluQWNjb3VudE51bWJlckxpa2VDb2x1bW5JbmRleCIgaW50ZWdlcixcblx0IkluQWNjb3VudE51bWJlckxpa2VIZWFkZXJWYWx1ZSIgY2hhcmFjdGVyIHZhcnlpbmcsXG5cdCJJbkFjY291bnROdW1iZXJMaWtlRmlyc3QiIGludGVnZXIsXG5cdCJJbkFjY291bnROdW1iZXJMaWtlTGFzdCIgaW50ZWdlcixcblx0IkluQWNjb3VudE51bWJlckxpa2VPcHRpb24iIGludGVnZXIsXG5cdCJJbkFjY291bnROdW1iZXJSZXF1aXJlZCIgYm9vbGVhbixcblx0IkluVXNlcklkIiBjaGFyYWN0ZXIgdmFyeWluZylcbiAgICBSRVRVUk5TICJGZWVkcyIuIlJldHVyblN0YXR1cyJcbiAgICBMQU5HVUFHRSAncGxwZ3NxbCdcbiAgICBDT1NUIDEwMFxuICAgIFZPTEFUSUxFIFBBUkFMTEVMIFVOU0FGRVxuQVMgJEJPRFkkXG5cbkRFQ0xBUkUgIlJldHVyblJlY29yZCIgIkZlZWRzIi4iUmV0dXJuU3RhdHVzIjsgICBcbkRFQ0xBUkUgIlJlc3VsdFJldHVyblJlY29yZCIgIkZlZWRzIi4iUmV0dXJuU3RhdHVzIjsgICBcbkRFQ0xBUkUgIkFjY291bnROdW1iZXJSZWdpc3RyYXRpb25SZWNvcmQiIFJFQ09SRDsgXG5ERUNMQVJFICJTdGF0dXMiIGNoYXJhY3RlciB2YXJ5aW5nIDo9J0ZhaWxlZCc7ICBcbkRFQ0xBUkUgIlFyeSIgY2hhcmFjdGVyIHZhcnlpbmcgOj0nJzsgIFxuREVDTEFSRSAiRmllbGROYW1lIiBjaGFyYWN0ZXIgdmFyeWluZyA6PScnOyBcbkRFQ0xBUkUgIkxpa2VGaWVsZE5hbWUiIGNoYXJhY3RlciB2YXJ5aW5nIDo9Jyc7ICAgXG5ERUNMQVJFICJEb25lIiBib29sIDo9ZmFsc2U7ICAgXG5kZWNsYXJlICJMb2NhdGlvbk5vdFNwZWNpZmllZCIgaW50ZWdlcjo9LTE7XG5kZWNsYXJlICJMb2NhdGlvbkZ1bGxWYWx1ZSIgaW50ZWdlcjo9MDtcbmRlY2xhcmUgIkxvY2F0aW9uRmlyc3ROQ2hhcmFjdGVycyIgaW50ZWdlcjo9MTtcbmRlY2xhcmUgIkxvY2F0aW9uTGFzdE5DaGFyYWN0ZXJzIiBpbnRlZ2VyOj0yO1xuZGVjbGFyZSAiTG9jYXRpb25CZXR3ZWVuRmlyc3RBbmRMYXN0Q2hhcmFjdGVycyIgaW50ZWdlcjo9MzsgIFxuZGVjbGFyZSAiTG9jYXRpb25NYXNrIiBpbnRlZ2VyOj00OyAgXG5kZWNsYXJlICJTdGVwIiBpbnRlZ2VyIDo9MDtcbkJFR0lOXG5cdCJSZXR1cm5SZWNvcmQiLiJNZXNzYWdlIj0nJztcblx0IlJldHVyblJlY29yZCIuIlN1Y2Nlc3NmdWwiPXRydWU7XG5cdC8qKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKi9cblx0LyogVGhlIHZhbHVlIG9mIEFjY291bnQgTnVtYmVyIGlzIG9ubHkgdXNlZCAgICAqL1xuXHQvKiB3aGVuIHJlc29sdmluZyB0aGUgQWNjb3VudCBOdW1iZXIgaWYgdGhlcmUgICovXG5cdC8qIHRoZSBzYW1lIGFjY291bnQgbnVtYmVyIGV4aXN0cyBpbiBtb3JlIHRoYW4gKi9cblx0Lyogb25lIEFjY291bnQgTnVtYmVyLiAgICAgICAgICAgICAgICAgICAgICAgICAqLyAgXG5cdC8qKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKi9cblx0IkluVXBsb2FkRmlsZVJlZ2lzdHJhdGlvbklkIj1jb2FsZXNjZSgiSW5VcGxvYWRGaWxlUmVnaXN0cmF0aW9uSWQiLDApOyAgIFxuXHQiSW5BY2NvdW50UmVnaXN0cmF0aW9uSWQiPWNvYWxlc2NlKCJJbkFjY291bnRSZWdpc3RyYXRpb25JZCIsMCk7IFxuXHQiSW5BY2NvdW50TnVtYmVyQ29sdW1uSW5kZXgiPWNvYWxlc2NlKCJJbkFjY291bnROdW1iZXJDb2x1bW5JbmRleCIsLTEpOyAgXG5cdCJJbkFjY291bnROdW1iZXJIZWFkZXJWYWx1ZSI9Y29hbGVzY2UoIkluQWNjb3VudE51bWJlckhlYWRlclZhbHVlIiwnJyk7ICBcblx0IkluQWNjb3VudE51bWJlckxpa2VDb2x1bW5JbmRleCI9Y29hbGVzY2UoIkluQWNjb3VudE51bWJlckxpa2VDb2x1bW5JbmRleCIsLTEpOyAgXG5cdCJJbkFjY291bnROdW1iZXJMaWtlRmlyc3QiPWNvYWxlc2NlKCJJbkFjY291bnROdW1iZXJMaWtlRmlyc3QiLC0xKTsgIFxuXHQiSW5BY2NvdW50TnVtYmVyTGlrZUxhc3QiPWNvYWxlc2NlKCJJbkFjY291bnROdW1iZXJMaWtlTGFzdCIsLTEpOyAgXG5cdCJJbkFjY291bnROdW1iZXJMaWtlSGVhZGVyVmFsdWUiPWNvYWxlc2NlKCJJbkFjY291bnROdW1iZXJMaWtlSGVhZGVyVmFsdWUiLCcnKTsgICAgXG4gXHQvKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqL1xuIFx0LyogVXNlIEFjY291bnQgTnVtYmVyIFJlZ2lzdHJhdGlvbiBJZCB3aGVuID4gMFx0ICAgKi9cbiBcdC8qKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKiovXG5cdCJTdGVwIj0iU3RlcCIrMTtcbiBcdGlmKCJEb25lIiA9IGZhbHNlKSB0aGVuICAgXG4gIFx0XHRpZigiSW5BY2NvdW50UmVnaXN0cmF0aW9uSWQiID4gMCkgdGhlbiAgXG4gIFx0XHRcdCJEb25lIj10cnVlOyBcbiBcdFx0XHQgc2VsZWN0ICogSU5UTyAiQWNjb3VudE51bWJlclJlZ2lzdHJhdGlvblJlY29yZCJcbiBcdFx0XHQgZnJvbSAiSW52ZXN0Ii4iQWNjb3VudFJlZ2lzdHJhdGlvbiIgQVxuIFx0XHRcdCB3aGVyZSBBLiJBY2NvdW50TnVtYmVyUmVnaXN0cmF0aW9uSWQiID0gIkluQWNjb3VudFJlZ2lzdHJhdGlvbklkIlxuIFx0XHRcdDtcbiBcdFx0XHRpZigiQWNjb3VudE51bWJlclJlZ2lzdHJhdGlvblJlY29yZCIgaXMgbnVsbCkgdGhlblxuIFx0XHRcdFx0IlJldHVyblJlY29yZCIuIk1lc3NhZ2UiPWNvbmNhdCgiUmV0dXJuUmVjb3JkIi4iTWVzc2FnZSIsXG4gXHRcdFx0XHRcdCdBY2NvdW50IFJlZ2lzdHJhdGlvbiBJZCAoJywiSW5BY2NvdW50UmVnaXN0cmF0aW9uSWQiLCcpICcsXG4gXHRcdFx0XHRcdCcgbm90IGZvdW5kLiBSZXZpZXcgTGF5b3V0IHNwZWNpZmljYXRpb24uPGJyLz4nKTsgIFxuIFx0XHRcdFx0IlJldHVyblJlY29yZCIuIlN1Y2Nlc3NmdWwiPWZhbHNlOyBcbiBcdFx0XHRlbHNlIFxuIFx0XHRcdFx0aWYoIkFjY291bnROdW1iZXJSZWdpc3RyYXRpb25SZWNvcmQiLiJEZWxldGVUcyIgaXMgbm90IG51bGwpIHRoZW5cbiBcdFx0XHRcdFx0IlJldHVyblJlY29yZCIuIk1lc3NhZ2UiPWNvbmNhdCgiUmV0dXJuUmVjb3JkIi4iTWVzc2FnZSIsXG4gXHRcdFx0XHRcdFx0J0FjY291bnQgUmVnaXN0cmF0aW9uIElkICgnLCJJbkFjY291bnRSZWdpc3RyYXRpb25JZCIsJykgJyxcbiBcdFx0XHRcdFx0XHQnIGlzIG1hcmtlZCBmb3IgZGVsZXRpb24uPGJyLz4nKTsgIFxuIFx0XHRcdFx0XHQiUmV0dXJuUmVjb3JkIi4iU3VjY2Vzc2Z1bCI9ZmFsc2U7IFxuXHRcdFx0XHRcdCJEb25lIj10cnVlO1xuIFx0XHRcdFx0ZWxzZSAgXG4gXHRcdFx0XHRcdHVwZGF0ZSAiRmVlZHMiLiJVcGxvYWRUcmFuc2FjdGlvbkRhdGEiIEEgXG5cdFx0XHRcdFx0XHRzZXQgICJBY2NvdW50TnVtYmVyUGFyc2VkIj0iQWNjb3VudE51bWJlclJlZ2lzdHJhdGlvblJlY29yZCIuIkFjY291bnROdW1iZXIiLFxuXHRcdFx0XHRcdFx0XHQgIkFjY291bnROdW1iZXIiPWNvYWxlc2NlKCJBY2NvdW50TnVtYmVyUmVnaXN0cmF0aW9uUmVjb3JkIi4iQWNjb3VudE51bWJlciIsJycpLFxuXHRcdFx0XHRcdFx0XHQgIkFjY291bnRSZWdpc3RyYXRpb25JZCI9IkFjY291bnROdW1iZXJSZWdpc3RyYXRpb25SZWNvcmQiLiJBY2NvdW50UmVnaXN0cmF0aW9uSWQiLFxuXHRcdFx0XHRcdFx0XHQgIkJyb2tlcmFnZUZpcm0iPSJBY2NvdW50TnVtYmVyUmVnaXN0cmF0aW9uUmVjb3JkIi4iQnJva2VyYWdlRmlybSIsXG5cdFx0XHRcdFx0XHRcdCAiQnJva2VyYWdlRmlybVJlZ2lzdHJhdGlvbklkIj0iQWNjb3VudE51bWJlclJlZ2lzdHJhdGlvblJlY29yZCIuIkJyb2tlcmFnZUZpcm1SZWdpc3RyYXRpb25JZCIgXG5cdFx0XHRcdFx0d2hlcmUgQS4iVXBsb2FkRmlsZVJlZ2lzdHJhdGlvbklkIiA9ICJJblVwbG9hZEZpbGVSZWdpc3RyYXRpb25JZCJcbiBcdFx0XHRcdFx0XHRhbmQgQS4iUmVjb3JkVHlwZSIgPSAnRCc7ICAgXG4gXHRcdFx0XHRlbmQgaWY7XG4gXHRcdFx0ZW5kIGlmO1xuXHQgIFx0ZW5kIGlmO1xuIFx0ZW5kIGlmOyAgXG5SRVRVUk4gIlJldHVyblJlY29yZCI7IFxuRVhDRVBUSU9OIFdIRU4gT1RIRVJTIFRIRU5cblx0IlJldHVyblJlY29yZCIuIlN1Y2Nlc3NmdWwiPWZhbHNlO1xuXHQiUmV0dXJuUmVjb3JkIi4iTWVzc2FnZSI9Y29uY2F0KCJSZXR1cm5SZWNvcmQiLiJNZXNzYWdlIiwnRXhjZXB0aW9uIGluIFRyYW5zYWN0aW9uIERhdGEgQWNjb3VudE51bWJlciA6ICcsXG5cdFx0XHRcdFNRTEVSUk0sJzxici8+JyxTUUxTVEFURSwnPGJyLz4nLCJRcnkiLCc8YnIvPlN0ZXAgKCcsIlN0ZXAiLCcpPGJyLz4nKTsgXG5cdFJFVFVSTiAiUmV0dXJuUmVjb3JkIjtcbkVORDtcbiRCT0RZJDtcblxuQUxURVIgRlVOQ1RJT04gIkZlZWRzIi4iVHJhbnNhY3Rpb25EYXRhQWNjb3VudE51bWJlciIoYmlnaW50LCBiaWdpbnQsIGludGVnZXIsIGNoYXJhY3RlciB2YXJ5aW5nLCBpbnRlZ2VyLCBjaGFyYWN0ZXIgdmFyeWluZywgaW50ZWdlciwgaW50ZWdlciwgaW50ZWdlciwgYm9vbGVhbiwgY2hhcmFjdGVyIHZhcnlpbmcpXG4gICAgT1dORVIgVE8gcG9zdGdyZXM7XG5gLAogIGV4dGVuc2lvbnM6IFtiYXNpY1NldHVwLAogICAgICAgICAgICAgICAgc3FsKHsKICAgIGRpYWxlY3Q6IFBvc3RncmVTUUwsCiAgfSksCiAgICAgICAgICAgICAgXSwKICBwYXJlbnQ6IGRvY3VtZW50LmJvZHksCn0pCg==

2

Have you considered telling us what you find incorrect about the folding behavior?

3

I’m sorry I missed. So if I collapse BEGIN END block, it doesn’t collapse till the END keyword. Similarly, if I collapse a nested if block but doesn’t match. Please find the below screenshots. You can also try using the link.

image
image982×676 102 KB

image
image1190×1106 136 KB

4

Ah I see what you mean now. The SQL parser doesn’t really know SQL syntax, only tokens, and will fold any ‘statement’ that ends in a semicolon. In the constructs you are using there, that doesn’t work at all. SQL constructs are unfortunately pretty complicated and wildly different between different dialects, so properly parsing them isn’t something I want to commit to. There might be some heuristics we could use to improve stuff, but I’m not familiar enough with modern SQL to really know.

5

I can may be try creating custom folding. I’m guessing it should be part of PostgreSQL dialect. If you can give me some pointers where I can make changes then it would be helpful.

6

Hi @marijn,

I’m attaching the cold folding we used for CodeMirror 5 which kind of worked except nested blocks. May be this is a good starting point.
pgadmin-sqlfoldcode.js (4.5 KB)

7

Hi,

I can work on a PR if you guide me a bit. It will help a lot pgAdmin users.

8

If I had a good idea on how to do this, I would probably already have implemented it. But given that this is syntax the parser doesn’t recognize, I’m not sure how to derive folding information for it in a general way.

9

Can’t a dialect provide custom parsing or codefolding? Or is there any way to override code folding?

10

You can use foldService to provide custom, non-syntax-tree-based fold ranges if you want.

11

Hi @marijn,

Will creating a separate grammar for PostgreSQL work just like lang-sql/src/sql.grammar at main · codemirror/lang-sql · GitHub?

It would be good to get this correct for a lot of pgAdmin users. Please guide me.

12

It would have to be a lot more complex. That one is basically just a tokenizer, doesn’t recognize any actual syntax beyond that.

13

Hi,
I was wondering if we could add custom foldNodeProp to define folding. For that we’ll also need to define more types here. Can’t we do something like HTML tags which has opening and closing?

@top Script {
  Statement { element* Semi }*
  Statement { element+ }?
}