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.

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

Simplify line length #734

Merged
merged 4 commits into from
Feb 3, 2021
Merged

Simplify line length #734

merged 4 commits into from
Feb 3, 2021

Conversation

MichaelChirico
Copy link
Collaborator

No description provided.

I think it's safe to run this one on the full file only. `nchar` on 300K line script still takes <.1 seconds
Copy link
Collaborator

@AshesITR AshesITR left a comment

Choose a reason for hiding this comment

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

Just a quick thought: Wouldn't setting column_number = length + 1 be more convenient for breaking up the line?

@MichaelChirico
Copy link
Collaborator Author

MichaelChirico commented Feb 3, 2021

I think this is a more general issue... in my mind the ranges is used to highlight the lint:

a_really_long_line_according_to_20_characters
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

which if we go to column+1 will look odd for that use case...

vs the use case you have in mind, which is to land the cursor just after the lint when using it with the rstudio plugin.

Can we make these two use cases work together better?

@AshesITR
Copy link
Collaborator

AshesITR commented Feb 3, 2021

How would the highlight look if the range is left in tact (1 to nchar) and the column_number updated?

@MichaelChirico
Copy link
Collaborator Author

Oh sorry, I misunderstood. IIUC the ^~~~ tag is only built from ranges. So column_number should be fine to update. But perhaps best to keep as a follow-up to keep the PR more focused.

@AshesITR
Copy link
Collaborator

AshesITR commented Feb 3, 2021

Thats fine by me.

@AshesITR AshesITR merged commit c22e95e into master Feb 3, 2021
@AshesITR AshesITR deleted the simplify-line-length branch February 3, 2021 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants