src: Rebrand two lines of "ppm --version" output

[DeeDeeG]

We're not finding/printing the Atom version, we're finding/printing the Pulsar version. Rebrand this little bit of ppm --version.

Before:

% ./bin/ppm --version
apm  2.7.0
npm  6.14.19-pulsar1-1
node 16.0.0 x64
atom 1.109.2023092501
python 2.7.16
git 2.39.2

apm 2.7.0
[ . . . ]
atom 1.109.2023092501

After:

% ./bin/ppm --version
ppm  2.7.0
npm  6.14.19-pulsar1-1
node 16.0.0 x64
pulsar 1.109.2023092501
python 2.7.16
git 2.39.2

ppm 2.7.0
[ . . . ]
pulsar 1.109.2023092501

[DeeDeeG]

I also considered renaming the function getAtomVersion() to getPulsarVersion() and the var atomVersion to pulsarVersion, since these are used only internally in src/apm-cli.js, and they're not exported.

I dunno, since there's so much "atom" this and "apm" that left in the code, I went for the more minimal change, but mighty tempted to rename the var and function names to be accurate to Pulsar, not the old un-rebranded "Atom" var/function names.

Not that it strictly matters, just maybe a bit less confusing to read to have to have a mental model that "Oh, yeah, then they say Atom they mean Pulsar, right..." Hmm.

EDIT: Likewise for the apmVersion var.

[confused-Techie]

Looks great to me! Something that's been totally glossed over.

But I'm on board with keeping the other variables the same for now, although would keep my approval if you did rename them.

Personally, I feel like we essentially always have to do the mental translation of Atom to Pulsar, that having it here doesn't add much more cognitive load. And really for myself, if we ever did go through and fix these names up, I feel like going with an agnostic variable like editor could be helpful just to keep things as simple as possible if any kind of rebrand ever occurred again. But that's just me

[DeeDeeG]

Hmm. About this JSON output.

Wondering if I need to stuff apm and atom into the JSON output, since that is presumably meant for programmatic use. I'm admittedly not familiar with how this JSON output is used, but wouldn't be at all surprised if this is relevant to, say, checking if the user's installed atomVersion is a supported version in a package-to-be-installed's engines field in its package.json?

Or this PR could possibly just leave the JSON output not re-branded at all, leave the interface for programmatic use totally 100% unchanged. (Adding new properties seems safe-ish to me, deleting atom and apm might be disruptive either to core editor if it's used there (not sure) and probably has some impact when interacting with the package backend??)

[DeeDeeG]

./bin/ppm install still works on this branch, though, as does ./bin/ppm install --json. So, I haven't found a place where this clearly breaks stuff with just trying ppm install so far.

ToDo: I guess I could build Pulsar locally with ppm carrying this change and see how broken (if at all) package management is in the core editor??

[confused-Techie]

You could do all of that, although if it were me, I'd opt to just adding new properties, if it is something you are worried about.

[2colours]

My request is the same as during the decaf process: please, don't make too many changes during the code redesign - or even better: if you do, please make the appropriate modification on the async branch as well. The PR should be writeable by the people who can merge.

[DeeDeeG]

@2colours if you want these changes cherry-picked to your branch 2colours:asyncify-without-top, the one used in PR #95, I can do that.

Or if you don't mind, I can merge master branch into that PR branch to sync them.

---

I think adding new params to the JSON output and leaving the old ones is the safest move, so I'm intending to go ahead and do that soon. I may real-world test it in production just to see what the impact is, however.

[2colours]

@DeeDeeG right now I'm already very much overdue with other stuff that I procrastinated - this ppm stuff was really useful for that purpose... - so I haven't actually checked the cherry-pick for this one. However, I did rebase when #99 got merged, and that one had to be completely remade, even though it was just a bunch of one-liners. I'm afraid this would be the case here as well; even if it's not a big change, probably it needs to be redone because of the conflicts, in which case trying to merge them one after the other would probably yield merge conflicts once more.

So I'm not sure if it's worth it.

I finalized the PR now, and even though the tests are all passing (actually, a couple more tests as well because I activated the test cases for apm-cli), I would be thankful to get some help with some real-life "integration testing" of it. That's probably more feasible than to try and read essentially the whole code.

[DeeDeeG]

If I do a cherry-pick or a merge commit into #95, either way would add this little change just to the end of your branch, so it would be at most one merge conflict and I can manually resolve it correctly since this one is a very simple tiny change.

Yeah, #99 #95 is large, thank you for the context and I think that matter can be addressed in the other PR #99 #95.

[DeeDeeG]

The re-enabled spec/apm-cli.js --> spec/apm-cli-spec.js tests are passing, and I re-added the old parameters apm: and atom: to the JSON output, just to be safe.

Merging on the basis of above reviews and discussion. I know there is new diff, but it seems in line with previous discussion and these changes are very tiny, and I've even tested making the changes in the copy of ppm bundled with Pulsar locally, and it doesn't seem to mind at all, even when I tested with removing the atom: and apm: from the JSON output it was fine. But leaving them in just to be conservative.