brigadier icon indicating copy to clipboard operation
brigadier copied to clipboard

Add some quality of life improvements to `StringReader`

Open Yurihaia opened this issue 5 years ago • 5 comments

Added 2 new useful functions to StringReader

  1. overload of StringReader::expect that accepts a string, and expects the string
  2. StringReader::expectOption which expects one of a list of String

Yurihaia avatar Sep 27 '18 15:09 Yurihaia

CLA assistant check
All CLA requirements met.

msftclas avatar Sep 27 '18 15:09 msftclas

There we go. I made all of the requested changes (I hope)

Yurihaia avatar Oct 04 '18 23:10 Yurihaia

Maybe it would be useful to have a expectOption overload which accepts some kind of Collection, for example List. Then the current implementation could call it with expectOption(Arrays.asList(opts)) which is not that expensive since the created list is backed by the array.

Marcono1234 avatar Oct 18 '18 23:10 Marcono1234

@Marcono1234 I'm not fond of any options that require you to wrap an array in a collection.

That said, I could see an overload that takes a collection being useful in some situations. My approach would be to make it as generalised as possible, though, so you can take any object you want, not just lists.

String expectOption(final Collection<String> opts);

String expectOption(final String... opts);
        expectOption("1", "2", "3");
        expectOption(new ArrayList<>());
        expectOption(new HashSet<>());

Sollace avatar Oct 19 '18 18:10 Sollace

@Dinnerbone Is there anything else I need to change for this to go through?

Yurihaia avatar Nov 10 '18 16:11 Yurihaia

Closing and reopening to rerun checks.

peterix avatar Oct 26 '22 15:10 peterix

OOF. I guess we cannot reopen. And we can't rerun checks without doing odd things.

Sorry :(

peterix avatar Oct 26 '22 15:10 peterix