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

Option to wrap at specified column only when window is larger than the specified number of columns #1490

Closed
hashhar opened this issue Dec 19, 2015 · 11 comments
Labels
feature-request Request for new features or functionality
Milestone

Comments

@hashhar
Copy link
Contributor

hashhar commented Dec 19, 2015

Consider this a feature request:

I am looking to be able to wrap code to 80 columns but only when my window is larger than 80 columns.
Otherwise I want it to use the viewport wrapping.

I guess this would be useful for people who have two windows open side by side and one of the windows has two files open side by side (settings.json for me). In such cases, I cannot view the file without scrolling sideways.

@egamma egamma added the feature-request Request for new features or functionality label Dec 20, 2015
@egamma egamma added this to the Backlog milestone Dec 20, 2015
@egamma egamma added the editor label Dec 20, 2015
@bpasero
Copy link
Member

bpasero commented Dec 23, 2015

@alexandrudima fyi

@Fania
Copy link

Fania commented May 8, 2016

Yes please!

@hashhar
Copy link
Contributor Author

hashhar commented May 8, 2016

I would like to start implementing this myself. Reading through the code myself and setting up the environment. Will ask if any help needed.

@hashhar
Copy link
Contributor Author

hashhar commented May 21, 2016

Hmmm, I was having some issues building it. But it's building fine now. I'll try and send a PR within a 5 or 6 days.

@hashhar
Copy link
Contributor Author

hashhar commented May 27, 2016

How can I find out the current viewport width? I think adding an extra conditional to src/vs/editor/common/config/commonEditorConfig.ts near line 117 would do the trick?

I'm sorry, this is turning out to be a bit more difficult than I expected. The codebase is really large.

@hashhar
Copy link
Contributor Author

hashhar commented May 27, 2016

Got it working. I now need to integrate it properly. @alexandrudima or @bpasero.

You can see the patch here. I have some notes in the commit message regarding the next steps to take to integrate it properly.

This is just a proof of concept, I am looking to add a new config setting which would be a boolean deciding if this behaviour would occur. And the value for the wrappingColumn (which I fixed to 80 right now) would then be read from the wrappingColumn property.

What do you think about this approach and if there could any improvements be made, please suggest.

@hashhar
Copy link
Contributor Author

hashhar commented May 30, 2016

@alexandrudima I am not able to get it to show in the default config file. How is the default config file generated?

Nevermind, found it. Now all that's left is the smoke test to see if I broke anything.

@hashhar
Copy link
Contributor Author

hashhar commented May 30, 2016

I'm gonna send a PR. It works on my end.

@hashhar hashhar mentioned this issue May 30, 2016
2 tasks
@Fania
Copy link

Fania commented Jun 6, 2016

Sounds great. Thanks for the effort!

@alexdima alexdima modified the milestones: August 2016, Backlog Aug 29, 2016
@alexdima
Copy link
Member

Just merged the PR from @hashhar. Thank you @hashhar!

@hashhar
Copy link
Contributor Author

hashhar commented Aug 29, 2016

Thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

5 participants