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

Integration scripts #2857

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Integration scripts #2857

wants to merge 5 commits into from

Conversation

daxgames
Copy link
Member

@daxgames daxgames commented Jul 4, 2023

@DRSDavidSoft New branch for us to work on integration scripts. It already has all the integration scripts in vendor/bin from the vagrant+packer branch.

Any changes to these should also be compatible with the vagrant+packer that will eventually be merged into master.

@DRSDavidSoft
Copy link
Contributor

Amazing stuff 👍🏻 Would you agree to moving some of the scripts to vendor\scripts instead of vendor\bin? Even vendor\bin\scripts would be nice!

@daxgames
Copy link
Member Author

daxgames commented Jul 4, 2023

The scripts are where they are so they would be in the path when running Cmder from the distribution package because they work for our processes as well as for end users wanting to add Cmder to other terminals.

Admittedly they are still in development and usefulness by end users is not fully proven yet.

The vagrant+packer branch may depend on a specific location so as long as they get moved in both branches so as not to break anything I guess it does not really matter.

@DRSDavidSoft
Copy link
Contributor

Great, I propose the following changes. If you aprove, I'll implement them at a later time.

  1. Move some of the scripts to /vendor/scripts and leave the others in /vendor/bin
  2. Update the other branch to reference the new paths
  3. Add /vendor/scripts to be in PATH the same as /vendor/bin

(For now, I propose that every script that starts with add- should be moved to /vendor/scripts).

Additionally, I'd like to add two new scripts:

  1. Adding to/removing from the context menu (like what the launcher does, I like your PowerShell approach more than the .exe one in integrations. Can also give the ability to check for presense)
  2. A script that would set the CMDER_ROOT environment variable
  3. A script akin to add-font.ps1 from Shark, a fork of Cmder that would allow the user to quickly wget some monospace fonts and install them

TBH, these scripts seem very useful to me! And I'm excited to getting them in the main branch. I hope you also agree with the additions that I'm going to work on.

Thanks, Dax!

@DRSDavidSoft
Copy link
Contributor

P.S. So sorry for being late on the reviewing of your Awesome PRs. I'm also very excited for your speed PR, I'd like to review it sooner so we can merge it for the next release. Much appreciated! 👍🏻

@daxgames
Copy link
Member Author

daxgames commented Jul 5, 2023

Therenis already a script that does #2.

@daxgames
Copy link
Member Author

daxgames commented Jul 5, 2023

Not sure the rationale for having two separate folders both added to the path when one folder added to the path does EXACTLY the same thing.

@DRSDavidSoft
Copy link
Contributor

Ah, thank you. I was thinking of keeping the /bin more clear of clutter, and to be clear I meant that /scripts should be added for PATH in the vagrant branch, as IMHO I'd prefer if the /scripts weren't in PATH for all users. I know adding more length to PATH is not the best idea, but I was thinking it'd be worth it to keep the /bin directory more clean. I hope you'd agree!

@daxgames
Copy link
Member Author

daxgames commented Jul 6, 2023

I'm not that opinionated about it, and they don't NEED to be in the path. Just trying to make it easy for users and I HATE typing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants