Why can't you package this up and publish on Melpa?

[wyuenho]

I know you've said in the README the nature of this package makes it difficult, but there's no explanation. There are lots of packages on Melpa that have multiple files, and the process for creating them is clearly documented in the Elisp manual and Melpa. Can you explain what difficulty you have?

[jscheid]

No worries, it's on the list.. in fact I've started working on it last week. But I do still think it's going to be difficult because of the "binary" blob... there shouldn't be any technical difficulties, but something tells me it'll be a somewhat drawn out process getting the Melpa guys to accept it. I might do a two step process - first support installation through Quelpa, then submit the PR for Melpa.

[wyuenho]

Why does the daemon server need to be in a base64 blob?

[aspiers]

@wyuenho commented on May 25, 2020 3:46 AM:

Why does the daemon server need to be in a base64 blob?

I'd love to know this too ;-)

[aspiers]

OK, I had a quick try at writing a Quelpa recipe:

(prettier-el :repo "jscheid/prettier.el"
             :fetcher github
             :files ("*.el" "*.js"
                     (:exclude "*-tests.el")))

but it failed in more than one way. One issue seems to be that quelpa can't figure out there's a dependency on iter2. I'm not sure if that's an issue with this repo putting the dependency in the ;; Package-Requires: line.

[wyuenho]

// quelpa can't figure out there's a dependency on iter2

If that's the case, that's a Quelpa bug and you should file it there. The Package-Requires header is the correct way to declare dependencies.

[aspiers]

OK, apparently the issue is the combination of this:

(defun package--description-file (dir)
  (concat (let ((subdir (file-name-nondirectory
                         (directory-file-name dir))))
            (if (string-match "\\([^.].*?\\)-\\([0-9]+\\(?:[.][0-9]+\\|\\(?:pre\\|beta\\|alpha\\)[0-9]+\\)*\\)" subdir)
                (match-string 1 subdir) subdir))
          "-pkg.el"))

and the fact that this repo has no file prettier-el-pkg.el. Rather than honouring prettier-pkg.el (and in particular its dependency on iter2), package.el sees fit to generate a new prettier-el-pkg.el which is missing the dependency.

[aspiers]

I can now confirm that renaming prettier-pkg.el to prettier-el-pkg.el suddenly makes quelpa install the missing dependencies.

[aspiers]

Yeah it could be a bug in Quelpa, actually. Looks like quelpa-build--build-multi-file-package does not read the headers of prettier.el because it does not match the package name I've given it (prettier-el, in an attempt to avoid conflicting with https://github.com/prettier/prettier-emacs, although with hindsight maybe it could be prettier).

[aspiers]

Ah, changing the Quelpa recipe name from prettier-el to prettier seems to fix it. So maybe not a bug.

[aspiers]

Regarding the need to execute makem.sh or Makefile targets prior to installation via quelpa / MELPA, the following seem relevant:

• https://github.com/melpa/melpa/issues/1486
• https://github.com/melpa/melpa/issues/1704#issuecomment-42683377
• https://github.com/melpa/melpa/issues/1776

[aspiers]

Based on that, two approaches spring to mind:

1. Build, test, and publish artefacts such as prettier-el.js.gz.base64 as part of a CI pipeline (e.g. GitHub triggering Travis CI or CircleCI), and then have the MELPA / Quelpa recipe pull the results from the published location instead of pulling only source files from this GitHub repo.
2. Add a post-installation command (e.g. M-x setup-prettier-mode) which sets up the build environment (yarn install etc.) and executes any compilation or other build tasks.

Maybe the first option is better given that:

• the "build/test" phases already exist in .github/workflows/test.yml
• it minimises the burden on users

[wyuenho]

I continue to question the need for a build step. Plenty of packages package non-elisp scripts as simple data files. I still don't understand why this package has to be so complicated.

[aspiers]

@wyuenho At very least, this package needs to run npm install or yarn install to get the Javascript module dependencies. Checking the node_modules/ tree into git would horrifically bloat the repository and goes against all best practices I've seen.

Additionally, the docstring for prettier--create-process says:

• The payload sent via stdin is base64-encoded with line breaks to ensure it can be sent via `tramp'.

• The payload is minified and gzip-compressed to help with startup time via `tramp' on slow, non-compressed connections.

which sounds very reasonable to me, although this could perhaps be made optional somehow.

In summary, I can't see any way to avoid a build step. Can you?

A third (and significant advantage) of doing the build/test in the CI pipeline is that it ensures a consistent build environment. This is far easier for the maintainer to support than the arbitrary build environments which would occur if build was done by the user via M-x setup-prettier-mode or similar.

[jscheid]

I still don't understand why this package has to be so complicated.

I think you mean why it's currently so complicated, from the perspective of trying to deploy it to MELPA. That's an easy one: I've built this package for myself, first and foremost, and performance and utility have been my top priorities. Making it easy to deploy to MELPA simply hasn't been a priority for me at any point so far.

Now, would it be nice if it was on MELPA? For sure. Does getting it on there have to be so complicated? Probably not, I'm sure there's lots of room for improvement.

Why does the daemon server need to be in a base64 blob?

1. I had decided at the time to link (or bundle, in JS lingo) diff-match-patch as that seemed the easiest way to get a good diff without/instead of
   • having to ask users to install it,
   • coming up with some homegrown way of linking the two pieces of code,
   • relying on external tools such as diff(1), which supports only line diffs and might not be present/functional.
2. Base64 helps paper over weird encoding issues that I've encountered with certain Tramp setups (yes, even without gzipping), but that's years ago so don't ask me about details. It doesn't matter though, as base64 encoding could be done at load time inside Emacs.

Using diff(1) instead of diff-match-patch might be a good compromise that would obviate the need for a separate build step, but first someone will have to check that it's not a performance regression or would cause other problems due to it being line-based. It's probably fine. It might be problematic for Windows users, who don't necessarily have a functional user land, although I don't know the platform well enough to know if that is still the case in recent versions.

[aspiers]

@jscheid Thanks a lot for the info! This all makes sense and is very helpful. And I just discovered your PR #13 which looks very promising. I'm currently trying this Quelpa/MELPA recipe which consumes that PR branch:

(prettier :repo "jscheid/prettier.el"
          :fetcher github
          :branch "melpa"
          :files ("*.el" "*.js" "*.base64" "dir" "*.info" "COPYING*"
                  (:exclude "*-tests.el" "*-pkg.el")))

although I excluded *-pkg.el due to a misunderstanding, before realising that the recipe needs to be named prettier not something else like prettier-el.

[jscheid]

Ah, awesome... I didn't get a chance yet to try with Quelpa, that would have been the next step. Let me know how you go. I'm afraid I'm quite busy with the day job and other commitments currently, and not entirely sure how soon I can focus on it again, but patches or just general feedback are always welcome.

[aspiers]

I think the Quelpa approach is fundamentally problematic right now, because IIUC it combines the downloading and packaging into a single step with no opportunity to execute build steps in between. Clearly this project currently needs build steps as discussed above and in #15, so until we can publish build artefacts automatically as part of the CI pipeline for Quelpa to download, I don't think Quelpa will work. And it probably won't work for installing from source, since if you've already downloaded the source and run yarn install, make etc. then Quelpa doesn't really add any extra value - at that point a simple use-package is good enough.

[wyuenho]

Re: build step: there are packages such as eslintd-fix and lsp-mode that offer the companion code as a npm module the user can install separately. In the case of lsp-mode, it will even automatically install the language server into user-directory on first launch. All you need to do is to put a pre-publish hook in your package.json to bundle with whatever bundler you want, and npm publish when you are done.

Re: Tramp and min+gzip: This whole issue can be sidestepped if you just publish an npm module, then you can use url-retrieve or request to communicate with the server.

The biggest advantage of this package is the persistent server process, every other micro optimizations are absolutely miniscule in comparison.

[jscheid]

Re: Tramp and min+gzip: This whole issue can be sidestepped if you just publish an npm module, then you can use url-retrieve or request to communicate with the server.

The biggest advantage of this package is the persistent server process, every other micro optimizations are absolutely miniscule in comparison.

If you truly believe that, I would suggest you install the official prettier-js Emacs package (from Melpa!); prettier_d; and (setq prettier-js-command "prettier_d").

I think, however, that you will find it quite a bit slower. The difference is not nearly as pronounced as that between prettier-js with and without prettier_d of course, without it's just silly slow. But there is still a significant difference, one that matters to me. If it doesn't matter to you, great -- there you go, you're all sorted.

Re: build step: there are packages such as eslintd-fix and lsp-mode that offer the companion code as a npm module the user can install separately.

That is not a bad idea at all, and one I will ponder, thank you. Not for a separate server process, just as a way to install the JS bundle.

[aspiers]

@jscheid commented on June 12, 2020 1:09 PM:

Re: Tramp and min+gzip: This whole issue can be sidestepped if you just publish an npm module, then you can use url-retrieve or request to communicate with the server.

[...]

Re: build step: there are packages such as eslintd-fix and lsp-mode that offer the companion code as a npm module the user can install separately.

That is not a bad idea at all, and one I will ponder, thank you. Not for a separate server process, just as a way to install the JS bundle.

Sounds great to me too!

[jscheid]

It's not without its own set of drawbacks, of course. Now you have to worry about versioning in one way or another; check the other package version; version the protocol, think about how/write code to handle version mismatches etc.

Uninstalling is messier. Upgrading isn't as straightforward anymore.

Still, we're deciding between the lesser of evils here so let's give it some thought.

[jscheid]

After putting some thought into it, I'm not convinced that a separate npm package is a good way forward.

The whole point of this ticket is to make it easier for users to manage (install, upgrade, and uninstall) the package. With a separate JS package, not only does each of those become a multi-step process, now the user has to worry about whether things are in sync. This seems antithetical to me.

I might bring up the issue with the Melpa maintainers and see if they have any ideas.

[wyuenho]

Is it hard to write a lisp function to check the npm package version and upgrade automatically? If you are installing the node module into user-directory, the lisp package can fully control that. While it's true you can't completely uninstall the npm module due to a lack of mechanism , does it really matter? Emacs itself leaves plenty of junk in user-directory anyway.

[wyuenho]

Besides, any improvement to the current installation/upgrade/uninstall process is an improvement. If you don't want to write an npm module, just check in the min gz base64 files into the repo in a different branch and you are done. Why complicate matters?

[jscheid]

@wyuenho would you be willing to maintain the Melpa presence of this package? All I would ask is that, at least for the time being, the structure is kept intact (like it is in the current release tarball) - beyond that it would be up to you how you'd structure the process. Checking in the min.gz.base64 for every release would be one way to do it, as you say.

[wyuenho]

You are already using Github Actions, you can just create a new workflow to connect the checkout action and the add commit action together right?

https://github.com/marketplace/actions/add-commit

[wyuenho]

In fact, this one looks even simpler yet

https://github.com/ad-m/github-push-action

[jscheid]

I agree that pushing into a separate branch looks like the best option. Do you want to implement it? We should create a new develop branch so that we can first test merged PRs without them going straight to Melpa.

[wyuenho]

create a new develop branch so that we can first test merged PRs without them going straight to Melpa

Allow me to quote Melpa's docs:

Packages in MELPA are built directly from the latest package source code in the upstream repositories, but we also build and publish packages corresponding to the latest tagged code in those repositories, where version tags exist.

https://github.com/melpa/melpa#melpa-stable

[jscheid]

Yes, but that page also says:

Most users should prefer MELPA over MELPA Stable.

Which means that most users will get whatever gets pushed to master. I would prefer to have a staging area so that there is a chance for me to dogfood a new version before it gets released to the public.

Thank you for the PR! I will test it in a separate repository. The final version will also have to build prettier.info and dir, and copy the diff-patch-match license. It should match the tarball contents. Will it push the latest version of prettier.el?

[wyuenho]

You can just check out the PR locally and test yourself or merge to whatever branch you want. This PR only builds when there are pushes to master. You can even set the default branch to develop on github so when people open a PR, it automatically targets the develop branch.

There's currently no info pages in master and nobody reads it. People just do a M-x package-list-packages, find prettier.el and click on the URL to open a browser to this github repo. Just keep the docs in the README up to date.

Incremental improvements and release early and often man, get agile :)

[wyuenho]

Also, WRT the license, just check it in... you'll never touch it again anyway

[aspiers]

@wyuenho commented on June 15, 2020 10:56 AM:

There's currently no info pages in master and nobody reads it. People just do a M-x package-list-packages, find prettier.el and click on the URL to open a browser to this github repo. Just keep the docs in the README up to date.

Incremental improvements and release early and often man, get agile :)

I agree wholeheartedly with this, even if I don't always practice what I preach!

[jscheid]

Meh, it's not that I intend to collect PRs in develop. It's that otherwise I will have to check out and build PRs locally for testing, and I'd rather not. It's a small amount of work now and will make things easier for me later.

Anyway, I'll take it from here, thanks again.

[wyuenho]

It's that otherwise I will have to check out and build PRs locally for testing

How else do you dog food? The current test workflow already runs the automated tests for PRs.