helix icon indicating copy to clipboard operation
helix copied to clipboard

feat: continue single-line comments

Open seanchen1991 opened this issue 2 years ago • 6 comments

Closes: #1730

This is a re-attempt at implementing continue-comment functionality, following the earlier attempt in #1937; I lifted most of @antoyo's logic from his initial PR.

This PR only implements continuation of single-line comments; block comments are left as a TODO. Additionally, I attempted to implement proper indentation in new comment lines as per this comment, but was finding it really finicky to get right, and so I'm opening this PR without proper handling of indentation in continued comments for now. I'll try to open a subsequent PR that adds that logic at a later time.

The functionality is behind a continue-comments configuration option that defaults to enabled. Additionally, a comment-tokens field was added for each language in languages.toml that accepts an array of strings in order to specify which comment tokens to look for. I think there's two ways to handle this: introduce the new comment-tokens field and deprecate the old comment-token field, or implement some logic that parses a string of comma-separated tokens in order to not introduce a breaking change. Which would be the preferred route here?

seanchen1991 avatar Oct 30 '23 14:10 seanchen1991

#4718 also adds support for multiple tokens. We will most likely merge that PR before we can give this a proper look.

If you're resubmitting work from another PR it's customary to give the original author credit, either by re-using their commits without changes or by giving them co-author on the commits where you re-use their code: https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors

the-mikedavis avatar Oct 30 '23 14:10 the-mikedavis

If you're resubmitting work from another PR it's customary to give the original author credit

Is there a way to apply these changes to commits retroactively?

seanchen1991 avatar Oct 30 '23 17:10 seanchen1991

See https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/changing-a-commit-message#amending-older-or-multiple-commit-messages

Rebasing / ammending old commits can be complicated though. We could instead add a co-authored-by tag at merge-time if we squash + merge

the-mikedavis avatar Oct 30 '23 20:10 the-mikedavis

We could instead add a co-authored-by tag at merge-time if we squash + merge

Sounds good to me 🙂

seanchen1991 avatar Oct 31 '23 14:10 seanchen1991

@seanchen1991 just a little reminder that the toogle-block-comments PR is merged if you haven't noticed it yet :)

TornaxO7 avatar Apr 08 '24 08:04 TornaxO7

This change seems to be quite far along, what additional work is necessary to get it merged?

edit: ah i see its continued in #10996

djugei avatar Aug 24 '24 09:08 djugei

Closing in favor of #10996

the-mikedavis avatar Oct 03 '24 17:10 the-mikedavis