[WIP] - Example code collaboration

[dsifford]

Running List of Changes

• Add .gitignore file for _build directory
• Add ESLint following airbnb style guide (with slight modifications)
• Create separate file for ES6 changes.

  Various notes as I walk through the code

• This line in the code is technically incorrect, but Firefox and Chrome will fix it for you. This should always evaluate to false because classList does not exist on HTMLNode. ClassList only exists on HTMLElement. Writing it instead like this is the correct form:

const sib = menuElement.parentElement.nextElementSibling;

• Variable declared without any var, let, or const keyword here
• Functions should each satisfy one job. Composing functions within functions is dangerous, hard to debug, and makes following the code super confusing. (Example here)
• Having functions who's names are nearly identical (differing only in plurality) is confusing. (Example: domRemoveCitation vs domRemoveCitations)
• There are several functions that serve to complete a small part of a larger purpose. It would be more readable (and easier to follow) if these were combined into one function (e.g. a single function to update the DOM after a change is made)
• TBH, I think this example fails the people who are trying to learn from it because it's too easy to get lost in all the DOM-related functions. There's just too much random, unnecessary logic going on to be able to understand the real issue, which is how does one take citeproc-js and use it for inserting inline-citations. Using react for this to manage the state would probably shrink the DOM functions by a minimum of 50%, but there will still be some boilerplate that people will have to understand before getting to the main issue. Perhaps making a minimal example as well would be best here.. Something that doesn't generate and destroy popup menus and isn't fancy. I'm thinking perhaps having a single line of text and above it showing the citation selection menu. Trigger the business logic with a simple onChange handler. Done.

  Moral of the story, and conclusion I came to tonight.

• I think I might abandon trying to rewrite your code and just write a minimal example that you can choose to either include or not in your documentation.

  Confusion Remains...

• I'm still not entirely sure what the purpose of all this logic here is. Can you explain to me what the goal is? It feels like it's overly complex for the sake of being overly complex.

function getCurrentCitationInfo() {
        debug('getCurrentCitationInfo()');
        var info = {
            pos: 0,
            citationID: null
        };
        var citemeNodes = document.getElementsByClassName('citeme');
        var citationIndex = 0;
        for (var i=0,ilen=citemeNodes.length;i<ilen;i++) {
            var node = citemeNodes[i];
            var sib = node.nextSibling;
            var hasCitationNode = false;
            if (sib && sib.classList && sib.classList.contains('citation')) {
                hasCitationNode = true;
            }
            if (node.firstChild) {
                info.pos = i;
                info.citationIndex = citationIndex;
                if (hasCitationNode) {
                    info.citationID = sib.getAttribute('id');
                }
                break;
            } else if (hasCitationNode) {
                citationIndex++;
            }
        }
        return info;
    }

is citationIndex any different than simply i in this case?

wont they all haveCitationNode since you're iterating an HTML collection OF citation nodes? What is the purpose of citationIndex?

[dsifford]

@fbennett Are you at all opposed to me using ES6 syntax (including classes) in your example? Might be able to enhance readability.

[fbennett]

Nope. Readability is good.

[dsifford]

Cool. I'm also flirting with the idea of adding ESLint (using airbnb's style guide since that seems to be the most popular style these days) to ensure uniformity. Objections? Alternatives?

[fbennett]

In your hands, go for it.

[dsifford]

@fbennett Got a small amount of progress done for the day. Let me know your thoughts whenever you get time. 👍

[fbennett]

This looks great. I didn't know that a stray comma after the last element of an array was permitted (really), and I can see how that makes code easier to manage. I'll keep looking over your shoulder, but the comments are spot on so far.

Thinking out loud ... two things that would bring it closer to a general tool for deployment would be: to set up the data-state save/retrieve functions (the things that touch localStorage) as pluggable hooks; and to also set the insert/edit menu widget as a plugin, and hang it off of the "citation" node (instead of setting it as a child of the "citeme" chevrons).

With the latter change, the posToCitationId/citationIdToPos mapping nonsense could go away---you would only need a lookup object with currently registered citation IDs, which would reveal which is the new one being inserted. With all references to "citeme" nodes gone from the module code, it should be possible to run it in contexts other than the demo.

(About attribution, you've put significant work into this already, so feel free to replace my name with yours in the package [or add it, if that is more comfortable]). There is a license on the CSL processor itself, but this demo code was just done to help get the ball rolling. It's up for grabs, and I'm delighted to see what you are doing with it.)

[dsifford]

@fbennett What are your thoughts Re: me instead focusing on a minimal example? (see original PR comment for details).

There's just too many moving parts in the current example to be able to understand how citeproc works.

Re: your comments related to posToCitationId/citationIdToPos -- I still have no idea what those two variables are for.

[fbennett]

All the more reason to eliminate them. :-)

We're thinking in the same direction. Ideally there would be a module with just the essentials that a programmer could extend. At the moment there is this other gunk in there that isn't portable and obscures the underlying logic.

[dsifford]

Ok I gotcha.. Cool, let me play around some more and see what I can do. Need to digest your example a bit more still before I fully understand what's happening

[fbennett]

If you decide at some point that you would like the citationIdToPos mapping stuff stripped out, say the word: I know what needs doing for that, it would just take a little time to set up. On May 10, 2016 02:26, "Derek Sifford" notifications@github.com wrote:

Ok I gotcha.. Cool, let me play around some more and see what I can do. Need to digest your example a bit more still before I fully understand what's happening

— You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub https://github.com/Juris-M/citeproc-js-docs/pull/1#issuecomment-217930834

[dsifford]

If you decide at some point that you would like the citationIdToPos mapping stuff stripped out, say the word: I know what needs doing for that, it would just take a little time to set up.

Trying to figure out how to add a new tab to the sidebar now.

I'd like to see if I can make this into a workable simple React app. That would be easiest (for me) to remove a majority of the DOM cruft and also to make it consumable by others.

Tried adding a new .rst file and adding it to the list in index.rst but it didn't seem to build

[dsifford]

Might end up just making a separate example project since using rst is foreign to me.

It'll take too long for me to weed out the DOM stuff from the current example. If I continue on this path, the only way for me to be able to do it is to document everything as it currently sits so I can fully understand what's going on (which will take no less than 3 weeks, if I work on it every single day), and then go back over it and rewrite some things.

[dsifford]

Sorry for the tons of messages here. Using this feed as a running list of questions as they come up.

Next question: What is a 'jurisdiction'?

[dsifford]

I can't seem to reasonably understand what the general purpose of getCitationSplits is? I don't know what a split is. Should I?

I have no idea what the purpose is of citationsPre or citationsPost.

[dsifford]

Gonna call it a day there. Brain hurts.

I'll pick it back up once I hear back on those questions.

[fbennett]

That would be a good choice - if the rst is getting in the way, perfectly ok to leave it behind. Horses for courses. On May 10, 2016 03:04, "Derek Sifford" notifications@github.com wrote:

Might end up just making a separate example project since using rst is foreign to me.

It'll take too long for me to weed out the DOM stuff from the current example.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/Juris-M/citeproc-js-docs/pull/1#issuecomment-217941202

[fbennett]

Might end up just making a separate example project since using rst is foreign to me.

That sounds like a good call.

[fbennett]

Next question: What is a 'jurisdiction'?

It's a life-changing extension to CSL implemented in the citeproc-js. :smiley: If you select the US and the UK case cites in the demo page, you'll see that they are formatted differently. For legal resources (legal_case, hearing, bill, legislation, and regulation), citation structure is a property of the jurisdiction, not the style. To cope with that, the jurisdiction is set from a controlled list of identifiers embedded in the Juris-M client, and the processor loads a jurisdiction-specific module if it can find one. That's what it does, but you don't need to worry with the complexity: for document editing, it's enough to implement the retrieveStyleModule() interface in the processor instance. The processor will take care of the rest.

[fbennett]

I have no idea what the purpose is of citationsPre or citationsPost.

See the line note above. This data is needed to convey the current document state to the processor. It's used in the processCitationCluster() call on the processor instance.

[fbennett]

Gonna call it a day there. Brain hurts.

I know the feeling! Dynamic updates are hard. (I suspect you could count the people who have wrestled the issues to the ground on one hand, maybe two.) There really is light at the end of that tunnel though!

[dsifford]

@fbennett Thanks for the clarifications!

I know the feeling! Dynamic updates are hard. (I suspect you could count the people who have wrestled the issues to the ground on one hand, maybe two.) There really is light at the end of that tunnel though!

Haha you said it! I think once I familiarize myself with the jargon / variable names, things will become easier.. I will perservere!! 😜

Thanks again for all your help on this. It goes without saying that I totally couldn't do any of this without you!

I'll dive back in tomorrow and see if I can make some progress. Re: separate example project -- would you prefer that in a separate repository or would you rather it be in a subdirectory of this one?

[fbennett]

Probably best as a separate project. When you're done, I can link across from the processor docs, and people can clone your work as a clean repo.

[dsifford]

Here's the repo. https://github.com/dsifford/citeprocjs-react

I suppose this PR is good to close now if you'd like

[fbennett]

Great - I'll follow the project. I've brought the 'draft' branch of citeproc-js-docs into line with 'master'. When you get a chance, could you direct this PR to 'draft'? (or generate another from your fork - I'm not sure how it works in GitHub) I'd like to do the refactoring I mentioned above, and maybe carry forward on code comments some after it's been simplified.

(But if it's a pain to change the PR target, it can stay as-is and I'll work locally until I have it working again.)

[dsifford]

@fbennett Sure thing... I'll close this one down now and resubmit

Still haven't had time to get started on the example project.. But will do that soon too!