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

Better split #11

Merged
merged 5 commits into from
Jul 26, 2021
Merged

Better split #11

merged 5 commits into from
Jul 26, 2021

Conversation

Joshix-1
Copy link
Contributor

@Joshix-1 Joshix-1 commented Jul 25, 2021

Breaking change!
VelenCommand#getShortcuts() now returns a String[] so it doesn't get changed later.

The command parsing alg is reworked to be more readable.

The args get parsed more intelligent with respect to escapes and double qoutes.

2021-07-25_23-27

@ShindouMihou ShindouMihou added this to the Next Version milestone Jul 26, 2021
@Joshix-1
Copy link
Contributor Author

I also added a new util method to allow named arguments like:
2021-07-25_23-27

@ShindouMihou
Copy link
Owner

I wonder if it would be better to make this a non-breaking change for now by adding the original methods but with a deprecated annotation instead unless I am missing a point here over "so it doesn't get changed later."? 🤔

@Joshix-1
Copy link
Contributor Author

Lists can change their length, so returning an array is better, because it shouldn't change.

@ShindouMihou
Copy link
Owner

I see, is this ready to go or do you want to add more since I will be pushing it to the master branch to place it on its own version?

@Joshix-1
Copy link
Contributor Author

Joshix-1 commented Jul 26, 2021

It's ready to go

@ShindouMihou
Copy link
Owner

Thanks for the PR, then! 🧇

@ShindouMihou ShindouMihou merged commit c5ce7a7 into ShindouMihou:master Jul 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants