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

New installation script for UNIX #469

Merged
merged 10 commits into from Nov 15, 2022
Merged

New installation script for UNIX #469

merged 10 commits into from Nov 15, 2022

Conversation

ghost
Copy link

@ghost ghost commented Nov 14, 2022

Hi, Ned here.
In this script, all the logic I used was production-ready and in active use. I guess there shouldn't be any problem with this one.

However, I still ask if you have a working UNIX (eg. macOS or Linux Distros) test this out. Any feedback is appreciated.

Features

  • Menu - Tiny & clean prompts to help new users customize their installation.
  • Git clone - Manually git clone the repo to provide support for older git versions.
  • Config - Automatically generate the config based on the user choices.
  • Colored logs - Provide an more easier on the eyes installation proccess with colorful log messages.
  • One liner usage - Give the possibility to install dein.vim with just one line for UNIX's users.
  • Tiny - The script size is less than 7.5K.

Examples

Test the new script by running one of the following commands:

# curl
sh -c "$(curl -fsSL https://raw.githubusercontent.com/santosned/dein.vim/master/bin/installer.sh)"
# or wget
sh -c "$(wget https://raw.githubusercontent.com/santosned/dein.vim/master/bin/installer.sh -O -)"

In some case users might wan't to keep the current vim config, so to handle that I added the --keep-config | -K arguments, which can be passed to the installation script, like this:

# curl
sh -c "$(curl -fsSL https://raw.githubusercontent.com/santosned/dein.vim/master/bin/installer.sh)" "" --keep-config

When needed this will allow the script to verify if the vim or nvim config exist, if true it'll generate the .vimrc config at the dein.vim base path, if false it'll overwrite any config if existent.

Issues

Closes #467

bin/installer.sh Outdated Show resolved Hide resolved
bin/installer.sh Outdated Show resolved Hide resolved
Copy link
Owner

@Shougo Shougo left a comment

Choose a reason for hiding this comment

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

I have reviewed.

E. M. Santos added 2 commits November 15, 2022 03:36
Fix wrong author name
Update version from 2.2 to 3.0
Remove unneeded code from initial config
Change two git commands with wrong project name
@ghost
Copy link
Author

ghost commented Nov 15, 2022

More than the changes requested, I took the time to clean every other mistake I found.

Now, it's good to go. 🎉

@ghost ghost requested a review from Shougo November 15, 2022 07:17
@Shougo
Copy link
Owner

Shougo commented Nov 15, 2022

Hm.. I don't like the default behavior is overwritten.
It breaks older behavior. It does not overwrite.

I think the overwrite is only if users want.

When needed this will allow the script to verify if the vim or nvim config exist, if true it'll generate the .vimrc config at the dein.vim base path, if false it'll overwrite any config if existent.

Copy link
Owner

@Shougo Shougo left a comment

Choose a reason for hiding this comment

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

keep_config behavior must be default.

bin/installer.sh Outdated
Comment on lines 222 to 227
while [ $# -gt 0 ]; do
case $1 in
--keep-config | -K) KEEP_CONFIG=yes ;;
esac
shift
done
Copy link
Owner

Choose a reason for hiding this comment

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

Please take dein installation directory argument for the compatibility.

@Shougo
Copy link
Owner

Shougo commented Nov 15, 2022

Please take dein installation directory argument for the compatibility.

Others are OK.

Copy link
Owner

@Shougo Shougo left a comment

Choose a reason for hiding this comment

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

Please take dein installation directory argument for the compatibility.

@Shougo
Copy link
Owner

Shougo commented Nov 15, 2022

And please fix README.md.

@ghost ghost requested a review from Shougo November 15, 2022 09:53
@ghost
Copy link
Author

ghost commented Nov 15, 2022

And please fix README.md.

I could do that. But in another PR, to keep everything clean and organized.

@Shougo
Copy link
Owner

Shougo commented Nov 15, 2022

OK. LGTM.

@Shougo Shougo merged commit 9db38a5 into Shougo:master Nov 15, 2022
@Shougo
Copy link
Owner

Shougo commented Nov 15, 2022

Merged. Thanks.

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.

Suggestion to improve installation process
1 participant