Skip to content

Conversation

ggrossetie
Copy link
Member

I'm using the same code on the Neo4j documentation UI.
Since this is a pure function, I wrote unit tests (see: https://github.com/neo4j-documentation/docs-ui/blob/master/src/specs/test.code.js) but I'm not sure how to automate them in the Gulp build.
Have you already thought about testing in the docs UI?

@mojavelinux
Copy link
Member

mojavelinux commented Dec 9, 2020

I think this can be reduced to a single rx. (even simpler if we had matchAll, but c'est la vie)

if (code.dataset.lang === 'console' && text.startsWith('$ ')) {
  var cmdRx = /^\$ \S[^\\\n]*(\\\n(?!\$ )[^\\\n]*)*(?=\n|$)/gm
  var cleanupRx = /( )? *\\\n */g
  var cmds = []
  var m
  while ((m = cmdRx.exec(text))) cmds.push(m[0].substr(2).replace(cleanupRx, '$1'))
  text = cmds.join('\n')
}

I don't think the commands need to be separated by a semi-colon. And endline does the same thing and is easier for the user to go back to. We could keep the line continuations but in this code I decided to flatten them.

@mojavelinux
Copy link
Member

Have you already thought about testing in the docs UI?

Not yet. It's still a problem we need to solve in Antora.

@ggrossetie
Copy link
Member Author

I think this can be reduced to a single rx. (even simpler if we had matchAll, but c'est la vie)

I'm fine using a rx. I can copy and paste your code or you can directly push a commit on my branch I don't mind.

don't think the commands need to be separated by a semi-colon. And endline does the same thing and is easier for the user to go back to.

Yes but it will actually run the command on paste. I don't expect the command to run on paste and when it does I'm always a bit shocked/scared (because I like to read the command one last time before pressing "Enter") but it might be just me 😅

We could keep the line continuations but in this code I decided to flatten them.

I think it's fine 👍

Not yet. It's still a problem we need to solve in Antora.

I'm looking forward to it 😉

@ggrossetie
Copy link
Member Author

About testing, should I add something to unit test this logic?
Basically what I have in mind is to check if we are running in a Node environment (module.exports is available) and, if so, export a function named copyableCommand. I can then require the JavaScript file and write a unit test with Mocha + Chai.

@mojavelinux
Copy link
Member

I pushed an update. I like your point about the deferred execution. That makes sense.

@mojavelinux
Copy link
Member

I don't think adding testing for this function is really necessary. It's adding too much complexity to the build for something that is rarely going to change once we get it working. And there are far more important things to test, so I'd rather look at the testing problem as a whole rather than to patch something in.

I do like your idea about how to test these pure functions and it's something we should consider for an Antora UI test harness. Though I'm really not 100% comfortable until we are testing in the browser since that's where most of the problems originate.

@mojavelinux
Copy link
Member

Perhaps we should join on && instead of ;. If you do run the command, you don't want it to continue if something is failing. And why would we give them commands we expect to fail?

@mojavelinux mojavelinux changed the title Allow to copy multiple commands and command with line continuation resolves #36 allow to copy multiple commands and command with line continuation Dec 10, 2020
@ggrossetie
Copy link
Member Author

Perhaps we should join on && instead of ;. If you do run the command, you don't want it to continue if something is failing. And why would we give them commands we expect to fail?

I agree 👍

@mojavelinux mojavelinux merged commit fa78320 into asciidoctor:main Dec 10, 2020
@ggrossetie ggrossetie deleted the copy-clipboard-cmds branch December 11, 2020 07:45
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