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

Feature/markdownify #56

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jonocarroll
Copy link
Contributor

If applied, this commit will introduce the markdownify() addin which converts HTML to markdown syntax. Refer to changes in README for examples.

Why is this change being made? I'm migrating my blog and want to update these HTML tags into native markdown. I realised it was a a fairly straightforward regex operation but wanted to formalise it into a package. I haven't seen this functionality elsewhere, but that doesn't preclude that from being the case.

@yonicd
Copy link
Collaborator

yonicd commented Mar 2, 2019

cool addition.

could you please update the function to include the following so it can be integrated into the package

  • markdownifyr.R
    • binding function must have a trailing r, ie markdownifyr (connects with hot key functionality)
    • add an example (CRAN forces examples for the addins)
example roxygen2 header
#' @title Imager
#'
#' @description Convert the selected path into an embedded image
#'
#'
#' @return a markdown image link
#' @export
#' @importFrom rstudioapi getSourceEditorContext modifyRange
#' 
#' @examples 
#' \dontrun{
#' remedy_example( 
#'     c( "https://thinkr.fr/wp-content/uploads/2015/03/thinkR1.png"), 
#'     imager
#'     )
#' }
imager <- function() {

  • addins.dcf
    • update the binding field
  • opts.R
    • add element to hotkey field in opts.R. Name of element function name without the trailing r, ie markdownify.
    • add suggested hotkey for macOS, this is for when end users add hotkeys in bulk and use remedy::remedy_opts$get("hotkeys")
  • testthat
    • add unit tests in testthat
  • covrpage
    • run covrpage covrpage::covrpage(update_badge = TRUE,vignette = TRUE) locally since ci can't run the unit tests for addins.

@yonicd yonicd self-requested a review March 2, 2019 12:25
@ColinFay
Copy link
Member

ColinFay commented Mar 2, 2019

Very good idea indeed.

I have a more or less fully functional regex here : https://github.com/ColinFay/backyard/blob/master/R/html_to_markdown.R

We can mix your PR and the code from there so to include more html tags.

@jonocarroll
Copy link
Contributor Author

@yonicd cheers, can do. By 'update the binding field' do you mean because it should be markdownifyr? I'm proposing ALT+SHIFT+m as a hotkey since it's unused. OPTION+SHIFT+m for Mac, I guess, but I'll have to test that. I didn't add tests yet as it was an initial proposal but I can do that now.

@ColinFay neat! I was torn between bulk-processing of source and piecewise. I went with the latter as I tend to have embedded tweets and I don't think I want to convert that HTML. I currently have this set to only performing one conversion at a time, but perhaps reformatting a bulk section of text makes sense (with a warning about trying to do too much).

I'll iterate.

@yonicd
Copy link
Collaborator

yonicd commented Mar 2, 2019

yes. the binding field needs to be the function name (that's being updated with trailing r).

those hotkeys are good, it's a problem with so many to find one that makes sense and isnt in use. all the hotkeys in opts are mac centric, please dont mix with winOS.

thanks again

@jonocarroll
Copy link
Contributor Author

I'm not sure I could even test on windows... ugh. I'm coming from linux.

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.

3 participants