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

Added new features to script window #7871

Merged
merged 16 commits into from Sep 30, 2022
Merged

Added new features to script window #7871

merged 16 commits into from Sep 30, 2022

Conversation

@lloyddewit lloyddewit marked this pull request as draft September 11, 2022 08:35
@lloyddewit lloyddewit changed the title Scintilla net editor Added new features to script window Sep 11, 2022
@lloyddewit
Copy link
Contributor Author

@rdstern @Patowhiz
This PR uses the 'ScintillaNET' package. This package provides more advanced editor features than the Windows Forms 'TextBox' control. This allows us to add features such as syntax highlighting and line numbers. However, I don't know how well 'ScintillaNET' would run in the R-Instat Linux version.
Who would be the best person to test this PR branch in Linux?

@lloyddewit
Copy link
Contributor Author

lloyddewit commented Sep 20, 2022

@rdstern
I added some of the key editor features requested by @lilyclements :

  • Fixed width font
  • Syntax highlighting
  • Line numbers
  • Ensured that typical editor functions like cut/copy/paste/undo/redo/select etc. all work in a standard way with standard keyboard shortcuts
  • Improved right mouse click menu

If you agree, then I think this would be a good time to merge. I would implement the next improvements in a new PR.
Please could you test?

Possible next improvements:

  • make 'ctrl-enter' execute the R statement at the cursor (rather than just the line), this should fix the current bugs with multi-line statements
  • add tabbed windows
  • add auto close brackets, close quotes and indentation
  • add reformat option to tidy indentation and line breaks (including splitting long lines)
  • tidy up file open/save/save-as functionality
  • add auto-complete
  • find/replace dialog
  • display cursor row/column at bottom of editor
  • ensure commands on R-Instat 'Edit' menu work as expected in script window
  • ensure controls translate to French and other languages
  • update help files
  • test on Unix

Do you have a preference regarding which I should implement first (I can provide time estimates if needed)?

If you want to compare against RStudio, then the RStudio editor functionality is described below.

https://support.rstudio.com/hc/en-us/articles/200484448-Editing-and-Executing-Code-in-the-RStudio-IDE
https://support.rstudio.com/hc/en-us/articles/200710523-Navigating-Code-in-the-RStudio-IDE
https://support.rstudio.com/hc/en-us/articles/205273297-Code-Completion-in-the-RStudio-IDE
https://support.rstudio.com/hc/en-us/articles/200484568-Code-Folding-and-Sections-in-the-RStudio-IDE

@lloyddewit lloyddewit marked this pull request as ready for review September 20, 2022 10:56
Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

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

@lloyddewit this is looking very nice indeed. I have searched for something to complain about, but it seems to work fine!

@rdstern
Copy link
Collaborator

rdstern commented Sep 20, 2022

@lloyddewit I don't have strong views about the order of the items in the new pull request. So very happy for you to decide on the most efficient logical order.

@lloyddewit
Copy link
Contributor Author

This is just a note for myself that I could test the script window with code from issue #7889.

End Using
End Sub

Private Sub mnuCopy_Click(sender As Object, e As EventArgs) Handles mnuCopy.Click
If txtScript.SelectionLength > 0 Then
If txtScript.SelectedText.Length > 0 Then
CopyText()
Copy link
Collaborator

@N-thony N-thony Sep 26, 2022

Choose a reason for hiding this comment

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

txtScript.Copy() on line 204? to make it consistent with line 211 and get rid of CopyText()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I understand your point.
Copytext() is also called by frmMain. Therefore, we cannot delete it.
If we don't call CopyText() on line 204, then we also need to call EnableDisableButtons() which adds an extra line of duplicated code.
So on balance, I'll leave the above code unchanged. I hope that's OK.

@lloyddewit lloyddewit merged commit a2799ea into IDEMSInternational:master Sep 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add line numbers in the script window Using rtf editor for the R-Instat script window
3 participants