Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Miscellaneous fixes #4

Merged
merged 10 commits into from
Jul 23, 2024
Merged

Conversation

savetheclocktower
Copy link
Collaborator

There were lots of broken internal links, since the last incarnation of the manual had fewer pages and more headings.

I also moved the images into the repo and changed the passthrough rule, which should fix #3.

…plus lots of other fixes.
…for adding `pulsar` to one's path and setting up your system to build native modules.
@savetheclocktower
Copy link
Collaborator Author

This one's been updated again with more content for the “adding pulsar to your shell” and “setting up a native module build toolchain” pages.

Some of the stuff in there has been marked with TODOs because we need to fix a couple of things in the code, but even in its pending state I think this would be really useful to show to someone the next time they can't get x-terminal-reloaded to install on Windows.

Copy link
Member

@confused-Techie confused-Techie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, this is a fantastic PR!
With the obviously needed fixing of many links being great, I seriously love the lengths you've gone to to reduce any handholding that's needed with the more complex parts of our setup instructions. Ensuring to cover ever edge case, and not just the basic steps of how to get setup, but full options on troubleshooting for the most common issues.

Seriously impressive work. I've added a few comments here and there that I think should be addressed prior to merging, but overall super rad work

docs/customizing-pulsar/style-tweaks.md Outdated Show resolved Hide resolved
@@ -5,4 +5,10 @@ layout: summary.ejs

Community packages extend, customize, and improve the features of Pulsar. The authors of community packages are part of the amazing community that makes Pulsar such a great editor.

If you’ve already read the [Package guides](/package-guides) section, then you might be ready for a deeper understanding of some of the ideas that were covered in that section. If so, read on.
A typical package will perform exactly one of the tasks listed below, and occasionally may perform more than one:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While not necessary, I think it'd be a nice thing to note in a much more obvious way that while these are the typical use cases, that there are not at all that could be accomplished.

Maybe something after the list like:

But of course that's not all that a package can do, some package's have found unique problems to solve:
* Word & Character counts
* Showing the time in your editor
* Discord integration to show it as your current activity
* Terminal integration
* Linting your code with select tools

Maybe this list is better off linking to resources, but not sure if it's needed. Just want to make sure that we make it expressly obvious that there's very few limits to what a package can do

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intent of this section is not to say “a package will do one of these five things,” but rather to say “a package is the method of distribution for each of these five kinds of customization.”

I'm trying (and perhaps failing) to communicate that

  • my pulsar-language-svelte package
  • my vibrant-ink-redux package
  • my calculator-light-ui package
  • my symbol-provider-bookmarks package
  • and my inline-autocompletion-textmate package

are all distributed the same way even though they do completely different things. It's also unusual for a package to do more than one of these things, even though there's not much enforcement of this in Pulsar.

(For instance: I don't think a single package can function as both a UI theme and a syntax theme, since I think it'll show up in only one of the two theme pickers based on your metadata selection. But either kind of theme package can run code! Its main field in package.json is respected!)

Anyway, I'll add a different section above this about how much freedom you have when authoring packages.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that sounds awesome to me. I did understand you weren't saying it was a restriction, I just wanted to take the chance to re-iterate the freedom in capabilities.

But if you plan to add it elsewhere, nearby that's fine by me

docs/getting-started/adding-terminal-commands.md Outdated Show resolved Hide resolved

* You _must_ have **Visual Studio 2022 Build Tools** or **Visual Studio 2019 Build Tools** installed — or the full Visual Studio IDE of either version.
* Select whichever one you have installed and click on [[Modify]].
* You _must_ have “Desktop development with C++” checked. If it is not selected, select it, then click on the [[Modify]] button at the bottom right corner of the window. It will install new components.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* You _must_ have “Desktop development with C++” checked. If it is not selected, select it, then click on the [[Modify]] button at the bottom right corner of the window. It will install new components.
* You _must_ have “Desktop development with C++” checked. If it is not selected, select it, then click on the [[Modify]] button at the bottom right corner of the window. It will install new components.
* If your Windows version is something other than Windows 10, ensure you've also checked to install the "Windows 10 SDK"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're saying that Windows 11 users will need to install “Windows 10 SDK” just to be able to build a package like x-terminal-reloaded?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I recall, yes.

If memory serves there was a support request of someone attempting to setup their toolchain, and they were on Windows 11. But even after installing everything else it wouldn't work, and they had to install the Windows 10 SDK to proceed.

I'll see if I can double check this info, as I'm just relying on my memory, and the fact that I made sure to add that in the docs after that support interaction. But let me see if I can find a source

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, @savetheclocktower I've found my original source.

From this interaction, the user was required to install the "Windows 10 SDK" while they were on Windows 11. This looks to be when attempting to install Spiker's terminal package.

But, and apparently I've forgotten about this. But a while back when we bumped the node-gyp version we used, we enabled support of both the Windows 10 SDK and 11. So while my comment was true at one point it no longer is.

So my added docs are now inaccurate. May be a good idea to instead change them to install any SDK version. But if that's not your experience like you said before, it may not be needed and we can omit it entirely. Then if in the future we find some edge cases those can be updated as needed.

img/atom/environment-variables.png Outdated Show resolved Hide resolved
Copy link
Member

@confused-Techie confused-Techie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As all of my concerns have been addressed, this PR seems like it's good to go.

Super awesome work here, lets get it merged!

@confused-Techie confused-Techie merged commit 545f7a9 into pulsar-edit:main Jul 23, 2024
@savetheclocktower savetheclocktower deleted the apd-misc-2 branch July 23, 2024 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move images into this repo
2 participants