asciidoctor-vscode icon indicating copy to clipboard operation
asciidoctor-vscode copied to clipboard

Folding mix of Attributes and single-line comments

Open apupier opened this issue 2 years ago • 10 comments

It removes the possibility to have Folding range for the first group of attributes or first group of single-line comments. This is due to a limitation of VS Code which doesn't handle several folding ranges starting on the same line. This limitation lead me to add some non negligeable code complexity.

resolves #719

https://github.com/asciidoctor/asciidoctor-vscode/assets/1105127/5649fe55-47de-4265-b15c-c3d0047c6e6b

apupier avatar Jun 23 '23 00:06 apupier

Arguably, we could collapse the document header when we click on the document title. The downside is that you cannot collapse the entire file but is that really useful?

Would it simplify the logic?

ggrossetie avatar Jun 26 '23 16:06 ggrossetie

Arguably, we could collapse the document header when we click on the document title. The downside is that you cannot collapse the entire file but is that really useful?

I do not understand why it would interfere with collapsing the whole file.

I feel like what would be interesting would be to implement the folding for each section delimited by = and the various levels of ==...=.

apupier avatar Jun 27 '23 07:06 apupier

I believe that, currently, if you fold on the document title, it will fold the whole document, correct?

What I'm proposing is that if you fold on the document title, it will only fold the document header (i.e. document attributes, author line, etc...).

By doing that, my guess is that you can simplify the logic for attributes folding? You can fold a group of attributes until a comment or an empty line is found.

ggrossetie avatar Jun 27 '23 11:06 ggrossetie

I believe that, currently, if you fold on the document title, it will fold the whole document, correct?

https://github.com/asciidoctor/asciidoctor-vscode/assets/1105127/c894a12b-ce96-4421-b238-81d50874bfc5

no, there is no fold on the document title

apupier avatar Jun 29 '23 06:06 apupier

What I'm proposing is that if you fold on the document title, it will only fold the document header (i.e. document attributes, author line, etc...).

By doing that, my guess is that you can simplify the logic for attributes folding? You can fold a group of attributes until a comment or an empty line is found.

So implementing it this way will effectively simplify code but I think having the fold of a whole "titled section" is more interesting.

Another side thoughts, if a folding section is missing, end users are nto block and can create their own custom folding section https://code.visualstudio.com/docs/editor/codebasics#_fold-selection

apupier avatar Jun 29 '23 07:06 apupier

but I think having the fold of a whole "titled section" is more interesting.

What do you mean? My idea was to have a special fold on the document title (single =) but keep the existing behavior for section titles (two or more =).

Another side thoughts, if a folding section is missing, end users are nto block and can create their own custom folding section https://code.visualstudio.com/docs/editor/codebasics#_fold-selection

Oh I didn't know that! We should probably mention that in the documentation!

ggrossetie avatar Jul 02 '23 00:07 ggrossetie

I think I understand what's bothering you with my proposal. In the following case, we won't be able to fold the whole block of attributes/comments since my proposal is basically to fold until a comment is found.

this is a paragraph

:an-attribute1: value of attribute
:an-attribute2: value of attribute
// A single-line comment.
:an-attribute: value of attribute
// A second single-line comment.
:a-second-attribute: value of second attribute

this is another paragraph.

Having said that, I don't think this is a common use case. The document header is where we usually have a mix of comments and attributes. Attributes defined in the body of the document are "rare". It's also extremely uncommon to have a block of comments/attributes in the body.

So I think we should fold attributes until we found a comment or something that's not an attribute.

Does it make sense?

ggrossetie avatar Jul 03 '23 21:07 ggrossetie

I think it's difficult to decide because it depends 😄

In the example below, we most likely want to fold all the attribute entries + comment:

this is a paragraph

:an-attribute1: value of attribute
:an-attribute2: value of attribute
//:an-attribute3: value of attribute
:an-attribute4: value of attribute

this is a paragraph

However in the following example I would argue that we want to fold per "group":

= Embracing a new thing
// localization
:example-caption: Exemple
:tip-caption: Astuce
// reveal.js options
:revealjs_fragmentInURL: true
:revealjs_hash: true
:revealjs_controls: true
:revealjs_controlsLayout: edges
:revealjs_controlsTutorial: true
:revealjs_slideNumber: c/t
:revealjs_showSlideNumber: speaker
:revealjs_autoPlayMedia: true
:revealjs_totalTime: 2700
// style
:icons: font
:source-highlighter: highlight.js

content...

In other words, comments are used as headers/delimiters so it makes sense to fold the localization group, the reveal.js options group and the style group independently.

For reference, the Intellij IDEA extension does not seem to provide this feature.

@mojavelinux @ahus1 I would love your input on this issue 🥺

ggrossetie avatar Jul 08 '23 08:07 ggrossetie

My intuition says that groups of attribute entries would be folded behind a leading line comment (the line comment being the fold line). (I'm not sure if that's feasible to implement, but it would make sense to me as a user).

mojavelinux avatar Jul 08 '23 09:07 mojavelinux

My intuition says that groups of attribute entries would be folded behind a leading line comment (the line comment being the fold line).

I agree 👍🏻

ggrossetie avatar Jul 12 '23 10:07 ggrossetie