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

Add word wrap feature #12

Merged
merged 1 commit into from
Apr 24, 2014
Merged

Add word wrap feature #12

merged 1 commit into from
Apr 24, 2014

Conversation

ncarchedi
Copy link
Contributor

Hi Jeff:

I'm using shinyAce for a project of mine and word wrapping would really be useful. I don't know a lick of javascript, but I was able to hack this together based on your existing code and the ace how-to guide. It seems to work well on my Mac.

Please note that I haven't updated any documentation, since I'm running roxygen2 v4.0.0 and I can't seem to prevent it from rewriting all of your docs.

Comments/criticism welcome.

Cheers,
Nick

@ncarchedi
Copy link
Contributor Author

@trestletech -- I'm sure you're swamped with more important things. I'm just curious if you intend to merge this, since that will determine if I use the word wrap feature in my shiny app. Thanks!

@trestletech
Copy link
Owner

Hey! Sorry it took a bit to get back to you.

This looks great; thanks for adding it. I see just a couple of things that we'd need to add before I merge:

  1. You finished the R side of the update function with word wrapping, but we'll need to teach JavaScript what it means for word wrapping to be enabled. I'm not actually familiar with this feature in Ace, so I'm not sure what the call would be, but we'd just need to add a conditional like this one: https://github.com/trestletech/shinyAce/blob/master/inst/www/shinyAce.js#L43 to update word wrapping dynamically in the editor.
  2. We'll need to update the docs. I use roxygen2 for this which is bundled pretty cleanly into RStudio's Packaging workflow, if you happen to be using that. If you regenerate the docs, it will update a couple of man files with the new parameters you added so others will know about them.

If you have a chance to do these, I'd be happy to merge. If not, I'll try to find some time to do it, but it might not be this week. Ultimately I'd definitely be happy to put this feature in, though.

@trestletech trestletech merged commit 479982c into trestletech:master Apr 24, 2014
trestletech added a commit that referenced this pull request Apr 24, 2014
Closes #12.

Merge branch 'master' of github.com:ncarchedi/shinyAce into feature/word-wrap

Conflicts:
	R/ace-editor.R
@ncarchedi
Copy link
Contributor Author

@trestletech Thanks for the merge! Sorry I was slow to respond. I actually hacked together a solution yesterday on the JS side of things, but yours is way more elegant.

I never thanked you for putting shinyAce together in the first place. It's EXTREMELY useful for the project I'm working on.

Cheers,
Nick

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