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.