ai2html
ai2html copied to clipboard
newsdev
Contributions, configs, building
I'm curious in simplifying the config. In my branch I removed the multiple configs, which was really confusing when I first tried to use ai2html.
Does the NYT customize the script before giving it to folks? Or do yall need for the ai2html.js file in this repo to work out of the box?
I'm also curious if yall are interested in breaking this file up and adding a script to build it. It would make it much easier to work in this code and would also make it possible to build different versions for NYT and other organizations.
Simplification is a worthwhile goal, I agree. It would be nice to get rid of the double configuration system and come up with a better way to configure the tool for the NYTimes and other organizations.
We don't customize the script at all. To deploy it, we just copy ai2html.js from the repo to the Illustrator Scripts directory. To decide which settings to apply, the script currently looks for NYT-specific fonts to tell if it's running in a Times-specific context, then looks for files and directories that are specific to our CMS. It's crude but it seems to work ok.
You are proposing that we organize the code into a collection of modules that would be bundled together by a build tool, is that right? Have you thought about how the modules might be organized? Do you have thoughts about which module system or tooling would be a good choice for this project?
It also sounds like you're envisioning that organization-specific configuration would be done at build time and 'baked in' to the executable. I'm not sure how that would work in practice, can you give some more details?
Looking forward to hearing your suggestions.
Perhaps we just want a settings.yml in the same folder as ai2html.js, which would allow anyone to configure the settings as they please.
Thats good to know how you deploy it.
I was thinking something really simple for a build system, like concatenation. The JS engine in Illustrator is not well documented, so I'd be worried about using clever build systems that assume node or v8 or popular browsers.
Should be pretty easy to accomplish with cat and a bash script or make file or something in package.json.
We could adjust that script to output different versions with different config sections.
Happy to put together a proof of concept for you to take a look at.
Hi @ryanmark
I like Tom's suggestion of switching to an external settings file (and a single set of default settings).
I'm not sold on splitting up ai2html.js into pieces that could be mixed-and-matched. It's simpler for us not to have any build process, and easier to support the script if everyone is running the same file. I'm happy to reconsider, though, if you want to put together a proof of concept.
--mbloch
Here is a proof of concept:
https://github.com/voxmedia/ai2html/tree/build-system
My intention in breaking up the large file is not to make pieces that can be mixed and matched, but to attempt to reduce the amount of code you have to look through at one time to understand the flow.
I left the bulk of the original ai2html.js together in src/lib/functions.js but it's clear from the code comments it could easily be broken up into separate files with different concerns. I broke out the minified json2 lib into a separate file in src/lib as an example of this. The body of the main init code is in src/main.js.
The build script takes json files from config and generates a separate version of ai2html for each.
I changed the script extension to jsx, which seems to be what illustrator prefers, but doesn't seem to matter. Just to illustrate that it doesn't matter what the extension is anymore (cuz you don't edit it directly).
This branch won't work in it's current state, it'll require some adjustments. Just showing this as an idea to get feedback. If y'all like it, I'll make it functional.
With this approach, a new user can grab this repo, copy config/default.json to config/mynewsorg.json, make changes, run build and have a custom version. If the upstream version changes, it's simple to merge those changes in and rerun build.
Thanks for this, I'll have a look and get back to you with notes... it might be a few days.
Curious to know if this has progressed in any way. @mbloch, did you have time to take a look?
Hi! following up on this. Any thoughts on the code I prepared above, @mbloch ?
Some comments on your sample code, @ryanmark
Your build script addresses two problems with the current program: configuration and code readability. These are good areas to work on, for sure. Thank you for spending time on this :)
I feel that configurability is the more pressing issue. Moving the config options into external JSON files as you've done is an improvement. I think we can go even further (I moved my suggestions to a separate issue, #86).
If ai2html switches to YAML for configuration, we might want to either bake out versions of the script with different configurations included -- like what you've done -- or load configuration settings from external files at run time.
Code readability is important too, certainly. You put most of the code from the original script into the file "functions.js" (2626 lines), so still a big chunk of code. As you mentioned, "functions.js" could be split up into smaller files, but then contributors would be searching between multiple files to trace the flow of the program, which doesn't seem any easier to me than searching inside a single file. One improvement might be to assign all of the functions in one source code file to the same namespace object, so for example, to call the "map" function from the file "utils.js" you would go utils.map().
I'm already familiar with the current script, so I'm not the best one to judge if these changes would make the code much easier to follow. I hope some other people will weigh in.
I love the idea of having a runtime config file, it would make it much easier for anyone to take and use ai2html. However I could see how that might be more difficult for NYT folks to have to install the plugin and a config file.
Thoughts about code organization:
Source control works best with many files. More, smaller files means cleaner merges and fewer merge conflicts, and probably (conjecture warning) more contributions and PRs, fewer forks.
As someone who reads lots of other peoples code and I find it much much easier to understand when they organize their code into different files that are labeled usefully. Just like with an article, I should be able to get the gist of the thing right away and follow the signposts to read more about what i'm interested in.
When all the code is in a single file, there are no signposts and the lead is buried. I can't just start reading the code, I have to scan it all in order to find the most important part, then build my own mental model of the whole thing before I can track down the parts I'm interested in.
Breaking the code out into other files, even if it's something generic like functions.js and main.js, makes it easier for me to immediately understand by starting with the most important stuff and then having the option to dig into other minutiae of the program that I'm most interested in.
Breaking out the code also helps to hide stuff that few people will ever have to look at. The minified json2 library for example. Almost nobody will ever have to touch or look at that. Standard library functions like forEach, map, filter, etc.; Nobody should need to see the implementations of these functions because they're so well documented, and most Javascript developers already understand what they are and what they do.
This is just my 2 cents about organizing code, which I'll admit I love to do.
There are lots of forks of this repo and seemingly many other news orgs using it. Would love to see us find a way to funnel all that effort into a single shared project so we can all benefit.
Oh and regarding name-spacing functions - I think that can be a great idea, but does not address the concerns about reading and contributing that breaking up the single file does.
Hi @ryanmark,
If you're saying that namespacing is not relevant to code readability, I guess I disagree. If I see that a function is being called from a module namespace, then I know where to look to find the function's definition. And if I see a bare function call, it's useful if I can assume that the function is local to the current source code file. I would prefer organizing the code into proper modules (with their own internal scope) over copying a bunch of bare functions into the main body of the script.
Your other points are well taken.
(I hope that the configuration changes I'm pushing for will remove the need for many orgs to maintain their own versions of the project -- currently you need to fork just to add your own fonts.)
I'm happy to keep discussing the code organization issue. Based on an unscientific survey, I'd say that my colleagues are still not on board.
Cheers, mbloch
I totally agree with your point about name spacing. It can improve readability. I was saying however, that if you just namespace functions but keep all the code in one file, it does not address the other points I made above. I still have to look at and understand most the code in order to find the parts I'm looking for.
Thank you for your considerations. I will look forward to the changes to improve configurations and in the meantime keep our changes in a fork, like other folks.
I see, we misunderstood each other... I meant to suggest using namespacing together with splitting the script into separate files.
I'd like to get the configuration changes done first, then revisit how to organize the code better.
For followers of this issue, I've made some changes to ai2html configuration, discussed here: #86