Add recipe prettier

[wyuenho]

Brief summary of what the package does

Format source code with a dedicated prettier process. Great support for yarn pnp.

Direct link to the package repository

https://github.com/jscheid/prettier.el

Your association with the package

Contributor

Relevant communications with the upstream package maintainer

jscheid/prettier.el#11

Checklist

Please confirm with x:

• The package is released under a GPL-Compatible Free Software License.
• I've read CONTRIBUTING.org
• I've used the latest version of package-lint to check for packaging issues, and addressed its feedback
• My elisp byte-compiles cleanly
• M-x checkdoc is happy with my docstrings
• I've built and installed the package using the instructions in CONTRIBUTING.org
• I have confirmed some of these without doing them

[wyuenho]

Ah, not sure why CI failed.

[conao3]

Force push (rerun CI) makes better result. maybe.

[riscy]

Is my understanding correct that this is essentially a broader version of prettier-js? I mention it since there might be a collaboration opportunity with @rcoedo.

The other thing I'll mention is that there is a small but significant preference in MELPA for using MELPA's regular conventions and indexing packages with their default branch specified, not a release branch. MELPA already recognizes tagged commits as (stable) releases, and the latest commit as the unstable latest.

Otherwise, here's a quick first pass --

prettier.el

byte-compile (using Emacs 26.3):

• No issues!

checkdoc (using version 0.6.1):

• No issues!

package-lint (using version 20200616.2257):

• No issues!

Suggestions/experimental static checks:

• prettier.el#L689: It's safer to sharp-quote function names; use #'
• prettier.el#L623: Consider unless ... instead of when (not ...)
• prettier.el#L654: Consider unless ... instead of when (not ...)
• prettier.el#L25: Prefer https over http if possible (why?)

[jscheid]

Is my understanding correct that this is essentially a broader version of prettier-js?

I'd say it's different not just in breadth, here is a list of differences.

I mention it since there might be a collaboration opportunity with @rcoedo.

For background, in 2018 I had submitted a bunch of PRs to prettier-emacs but they were sitting for a month without reply. Only then did I decide to start work on this package.

@rcoedo has since offered for me to take over maintenance but I'm actually quite happy with the decision to start from scratch. It left me free to choose the approach that I thought was best and meant I didn't have to worry about backward compatibility. (Also, I'd rather not recycle the prettier-js package name as it's misleading today.)

The other thing I'll mention is that there is a small but significant preference in MELPA for using MELPA's regular conventions and indexing packages with their default branch specified, not a release branch.

We have a build step that I would like to do without. I've considered several alternatives but after careful consideration, they all involve trade-offs I'd rather not make. That's why there is a separate branch for MELPA to pick up from, as per this comment. Is that not ok?

Otherwise, here's a quick first pass --

Thanks, fixed 🙂

[riscy]

Is that not ok?

It's fine if it's strictly necessary, but it's my duty to put downward pressure on the complexity of what goes into the recipes. :)

This looks like solid work, and an interesting approach to shaving some time off tramp.

[jscheid]

Thought so, makes sense. You guys are doing a stellar job with the curation, thanks for the merge and for all your careful work!

[tarsius]

I've just had a closer look at the differences.

They consist of:

1. Removal of many files. If we modify the recipe slightly, these removals make no difference as far as Melpa is concerned.

- :files (:defaults "*.js" "*.base64"))
+ :files (:defaults "bootstrap-min.js" "prettier-el.js.gz.base64"))

2. Addition of generated bootstrap-min.js and prettier-el.js.gz.base64. This appears to be the reason why you want to use a dedicated branch

3. Addition of generated/copied/? .nojekyll and COPYING-diff-match-patch. These are not relevant to Melpa and won't be included giving the current recipe.

4. Addition of generated dir and prettier.info. Melpa does not package the former, package.el generates it. The latter is generated using pandoc, which Melpa does not support.

Until I got to prettier.info, I was going to suggest that you find another way to distribute the javascript files -- that's what we usually ask maintainers to do in such situations. But because of the situation with the documentation, I'll refrain from asking you to do that.

At least now, we have a description of what's going on.

[jscheid]

@tarsius thanks for looking into this, appreciate it.

Just in case it wasn't obvious, the thread you're replying to is four years old and I've been distributing prettier via a branch for that long. This is all setup in CI and has been working fine all this time. Also, I'm planning to deprecate this package in favor of another one I've been working on, which doesn't include javascript, so no plans to touch any of this at the moment.

That being said, I would like to distribute an info file with the new package as well and looking for advice on best practices. I see that in magit you're using org files, checking in the generated texinfo files and then presumably Melpa picks it up from there? Is that an accurate description and can you recommend it? (I'd probably use pandoc again instead of org-texinfo-export-to-texinfo but that shouldn't matter if the texinfo gets commited.)

[tarsius]

the thread you're replying to is four years old

I was going through the list of the three packages that Melpa and the Emacsmirror get from diverging branches. I remove the other two packages for being deprecated resp. a complete mess, and was hoping this last remaining discrepancy could be resolved.

Also, I'm planning to deprecate this package in favor of another one I've been working on, which doesn't include javascript,

Great, when they are not completely necessary, it is nice to see dependencies on other languages (and their respective toolchains/ecosystems) be dropped.

... so no plans to touch any of this at the moment.

Once that other package exists, do you plan to deprecate this, maybe even archive it and remove it from Melpa?

I would like to distribute an info file

If that is an option, it would probably better to distribute a texi file.

I see that in magit you're using org files, checking in the generated texinfo files and then presumably Melpa picks it up from there? Is that an accurate description

Yes, Melpa generates the info file from the texi file. (And also dir, I misremembered that above, though it might still be accurate that package.el doesn't actually need that.)

... and can you recommend it?

Well. I would prefer if Melpa would take care of generating the texi from the org, and I even implemented support for that in package-build.el (the tool used by Melpa), but that would increase the attack surface, even when using bubblewrap as I did, so this was decided against.

So I have to continue to check the org and the texi files, and I am not particularly happy about that. I had worked for years on finally being able to stop doing that (getting changes into Org, elpa-admin.el, ..., and then failing to get the necessary changes adapted by Melpa).

I am considering to use a melpa branch myself, which is recreated on every push to main to add a single commit, which checks in the texi file(s). This branch would always be exactly one commit ahead; no merges.

So I can totally understand, if you stick to using a dedicated branch adding the generated files -- you probably won't be the only one for that much longer. 😁 But I would recommend using the approach just outlined, and name it melpa (or texi). My concern was more with the the generated javascript (there's an opportunity to obfuscate malicious code here), not the info file, and the use of a separate branch per se.

If you (unlike me) don't mind checking in the documentation twice on the main branch, then I would recommend doing that instead. It certainly is the simplest approach.

[jscheid]

Ah, I see now where you're coming from. All understood, and I commend you on your quest to do away with checked in generated code, in my opinion totally worth doing even if it sadly didn't lead anywhere.

And, more to the point, thanks a lot for working on improving the security of the ecosystem, which I gather is what you're doing here in the wake of the xz debacle.

I wasn't planning on deprecating the prettier package anytime soon because while there is a lot of overlap with the new package, it's not 100%. Also I'm not sure how soon my other package is going to be ready, it's just a little side project that I can barely find time for.

However what we could do is take off the base64 encoding, gzip, and minification and ship just plain JavaScript. Adding those layers was a micro-optimization aimed at slow TRAMP connections that in retrospect was a bit too fancy and we can do without really. Would that be an acceptable compromise?

[tarsius]

And, more to the point, thanks a lot for working on improving the security of the ecosystem, which I gather is what you're doing here in the wake of the xz debacle.

To be honest, the main motivation was to look into an unusual case, that requires special handling. But yes, I think we should avoid large binary-ish blobs, and stick to the source -- and security plays into that.

I wasn't planning on deprecating the prettier package anytime soon

That's find, I was just wondering.

However what we could do is take off the base64 encoding, gzip, and minification and ship just plain JavaScript. Adding those layers was a micro-optimization aimed at slow TRAMP connections that in retrospect was a bit too fancy and we can do without really. Would that be an acceptable compromise?

I am not sure it would be worth it. You would still require a "publish" branch (even if it weren't for the manual), right? You would still be turning many javascript files into one big file, but then stop there? Could the additional steps be performed after installation? Base64 encoding and gzipping should be possible without requiring a javascript tool chain -- minification, not so much. However, if you do that, you shouldn't do that in-place, modifying the package.el installed files (that would be even more "doing alarming things behind the users/systems back". Going down that road, things start to get more complicated, time consuming (to implement), and potentially fragile quickly.

Emacs's/package.el's whole "binary/foreign tools situation" isn't a solved problem. It makes sense for me to raise concerns, when I see a "problematic" case, but I don't want to demand any changes, especially not if a package has already been excepted into Melpa. (Of course, the ideal outcome would be if you looked into this more, came up with a universal solution, and got that merged into Emacs. 😀)

Maybe @riscy has a comment?