HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptModerate

Code-Q2A - copy code blocks from questions to answers

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
blocksanswersquestionsq2acodefromcopy

Problem

I came up with an idea for a userscript to simplify writing answers on Stack Exchange while writing a previous answer.

This userscript adds "Review"-links on the top of each code segment. When clicked, it adds checkboxes in front of every code line. You select the checkboxes for the lines you want to comment about, click the link again - which has now changed to "Add to answer", and voilà - the code will be copied to your answer.

I believe this userscript can significantly reduce the scrolling up and down where you consistently copy some code from the question, paste it in the answer, scroll up again, copy code, scroll down, paste, scroll up copy, etc... Now you can just go through the code from top to bottom and select the lines you want to comment on.

I have tested this userscript using Google Chrome + Tampermonkey, but I believe it should also work with Firefox + Greasemonkey, and with other browser/extension combinations that support userscripts.

The code is also available on GitHub.

If you are able to include a userscript from a link, use this link.

```
// ==UserScript==
// @name Auto-Review
// @author Simon Forsberg
// @namespace zomis
// @homepage https://www.github.com/Zomis/Auto-Review
// @description Adds checkboxes for copying code in a post to an answer.
// @include http://stackoverflow.com/*
// @include http://meta.stackoverflow.com/*
// @include http://superuser.com/*
// @include http://serverfault.com/*
// @include http://meta.superuser.com/*
// @include http://meta.serverfault.com/*
// @include http://stackapps.com/*
// @include http://askubuntu.com/*
// @include http://.stackexchange.com/
// @exclude http://chat.stackexchange.com/*
// ==/UserScript==

function embedFunction(name, theFunction) {
var script = document.createElement('script');
script.type = 'text/javascript';
script.textContent = theFunction.toString().replace(/function ?/, 'func

Solution

Naming

I realize naming can be hard, but 'Auto-Review' is a name that conflicts a bit with the exiting UserScript on StackExchange called AutoReviewComments

Protocols

There are a number of UserScript conventions you are failing to adhere to.

First up, for Firefox, you need to wrap the header section in to a "preserved" comment block:

/** @preserve
// ==UserScript==
.....
// ==/UserScript==
*/


The preserve is required to keep the comment block after the javascript is processed. This allows the 'compiled' version to keep the references, and for FireFox to know what it is about still.

Other browsers may not have the same requirements.

Additionally, you need to specify @grant permissions too for GreaseMonkey to be happy. In your case, none is appropriate:

// @grant   none


Once I made those modifications the userscript loaded up nicely in my FireFox.

Usability

There are four user-experience enhancements that I would suggest:

  • NO POPUPS - popups are a horrible distraction



  • Scroll-to-inserted-content - after inserting the code blocks, scroll to the edit point and make it visible on the screen



  • Trigger a changed-event on the answer entry box - this will update the 'preview' of the answer. As it is currently, you have to manually change something in the answer box for the preview to update.



  • You should clear the checkboxes after processing them. Having to un-check each box is a PITA if you need to copy different blocks later.



Review

-
double-array-dereferencing is a micro-optimization that's unnecessary. You have the following code (copied here using your userscript):

for (i = 0; i < checkboxes.length; i++) {
    if (!$(checkboxes[i]).prop('checked')) {
        continue;
    }

    var checkbox = $(checkboxes[i]);
    var line_data = (checkbox).data('line');


That code double-dereferences $(checkboxes[i]). I presume this is because you don't want to carry the overhead of the variable if it is not checked. This is an early-optimization. The code would be simpler as:

for (i = 0; i < checkboxes.length; i++) {
    var checkbox = $(checkboxes[i]);
    if (!checkbox.prop('checked')) {
        continue;
    }

    var line_data = (checkbox).data('line');


-
var declarations. JavaScript 'hoists' all variable declarations to the top of the function that contains it. Unlike other languages, JavaScript should not be coded with 'block-local' declarations. It is best practice to move all variable declarations to be the first items in the function. This Stack Overflow answer does a better job of explaining it than I would.

Code Snippets

/** @preserve
// ==UserScript==
.....
// ==/UserScript==
*/
// @grant   none
for (i = 0; i < checkboxes.length; i++) {
    if (!$(checkboxes[i]).prop('checked')) {
        continue;
    }

    var checkbox = $(checkboxes[i]);
    var line_data = (checkbox).data('line');
for (i = 0; i < checkboxes.length; i++) {
    var checkbox = $(checkboxes[i]);
    if (!checkbox.prop('checked')) {
        continue;
    }

    var line_data = (checkbox).data('line');

Context

StackExchange Code Review Q#78695, answer score: 12

Revisions (0)

No revisions yet.