Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add max_line_len to config.toml #2423

Closed
wants to merge 2 commits into from
Closed

Add max_line_len to config.toml #2423

wants to merge 2 commits into from

Conversation

fourlexboehm
Copy link
Contributor

I also thought this would be useful following issue #2393.
More than happy to change it if there's a better way.

@fourlexboehm fourlexboehm force-pushed the master branch 2 times, most recently from cf37651 to daf7d36 Compare May 8, 2022 10:21
pickfire
pickfire previously approved these changes May 9, 2022
View reviewed changes
Copy link
Contributor

@pickfire pickfire left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seemed reasonable to me. But I just realized the max-line-length config is not very discoverable from the website itself https://docs.helix-editor.com/configuration.html

archseer
archseer requested changes May 11, 2022
View reviewed changes
helix-view/src/editor.rs Outdated Show resolved Hide resolved
book/src/configuration.md Outdated Show resolved Hide resolved
@fourlexboehm fourlexboehm force-pushed the master branch 3 times, most recently from 58de43d to c87b8f8 Compare May 14, 2022 09:20
@fourlexboehm fourlexboehm requested a review from archseer May 14, 2022 09:42
@the-mikedavis the-mikedavis added the S-waiting-on-review Status: Awaiting review from a maintainer. label May 18, 2022
@kirawi kirawi added the A-helix-term Area: Helix term improvements label Sep 13, 2022
@oskarkook oskarkook mentioned this pull request Sep 26, 2022
Merged
the-mikedavis
the-mikedavis reviewed Oct 15, 2022
View reviewed changes
book/src/configuration.md
@@ -56,6 +56,7 @@ on unix operating systems.
| `rulers` | List of column positions at which to display the rulers. Can be overridden by language specific `rulers` in `languages.toml` file. | `[]` |
| `bufferline` | Renders a line at the top of the editor displaying open buffers. Can be `always`, `never` or `multiple` (only shown if more than one buffer is in use) | `never` |
| `color-modes` | Whether to color the mode indicator with different colors depending on the mode itself | `false` |
| `text-width` | Set the maximum line width for text wrapping with :reflow | `80` |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `text-width` | Set the maximum line width for text wrapping with :reflow | `80` |
| `text-width` | Set the maximum line width for text wrapping with `:reflow` | `80` |

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should also contain a reference to soft-wrap now that #5420 is merged

@the-mikedavis the-mikedavis added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. R-breaking-change This PR is a breaking change for some behavior and removed S-waiting-on-review Status: Awaiting review from a maintainer. labels Oct 15, 2022
@divarvel
Copy link
Contributor

divarvel commented Feb 9, 2023

Hi @fourlexboehm, I was considering working on something related now that the softwrap PR has landed (namely, adding a way to either use min(max-line-length,screen-size) for softwrapping (the current behaviour) or just screen-size. Since I'll be working on this area, i can pick up this PR and apply the remaining feedback items, and either PR to your branch or start a new branch and rebase your commits on top of a recent master. lmk what's best for you

@pascalkuthe
Copy link
Member

Hi @fourlexboehm, I was considering working on something related now that the softwrap PR has landed (namely, adding a way to either use min(max-line-length,screen-size) for softwrapping (the current behaviour) or just screen-size. Since I'll be working on this area, i can pick up this PR and apply the remaining feedback items, and either PR to your branch or start a new branch and rebase your commits on top of a recent master. lmk what's best for you

There has been no activity in this branch for 8 months, I think it's fair for you to take this over. I think it would be best if you rebase this on master. I think the original author should still stick around.

I am adding this to the next milestone because soft wrap is merged now and therefore this config option is now much more useful/widely used. Therefore I would like to make a breaking change like this before the next release. If you post your own PR I will add that to the milestone instead.

@pascalkuthe pascalkuthe added this to the next milestone Feb 9, 2023
@divarvel divarvel mentioned this pull request Feb 9, 2023
Merged
6 tasks
@pascalkuthe pascalkuthe removed this from the next milestone Feb 9, 2023
@divarvel
Copy link
Contributor

divarvel commented Feb 9, 2023

I'm picking up work here: #5893 (for now i've just cherry-picked the changes on top of a recent master and fixed a couple issues.

My plan was to add a config bit for having softwrap use text_width or not.

@pascalkuthe
Copy link
Member

Closing as redudant as #5893 has been merged which makes the same addition to config.toml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pascalkuthe pascalkuthe pascalkuthe left review comments

@the-mikedavis the-mikedavis the-mikedavis left review comments

@pickfire pickfire pickfire left review comments

@divarvel divarvel divarvel left review comments

@archseer archseer Awaiting requested review from archseer

Assignees
No one assigned
Labels
A-helix-term Area: Helix term improvements R-breaking-change This PR is a breaking change for some behavior S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@fourlexboehm @divarvel @pascalkuthe @archseer @pickfire @the-mikedavis @kirawi