Improve fault-tolerance of Modelica.Utilities.Strings.substring w.r.t. index arguments and remove undocumented case with endIndex=-999 #3503
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
beutlich
added
enhancement
beutlich
changed the title
HansOlsson
requested changes
Mar 15, 2020
There was a problem hiding this comment.
Good idea overall.
However,
- startIndex=0 is illegal, but not "negative" I would use the phrase "non-positive"
- I'm unsure if we totally should ignore messages for endIndex>length. Using startIndex=2 endIndex=9999999 (or a symbol for the latter) seems like a simple way to skip the first character.
beutlich
changed the title
beutlich
force-pushed
the
update-substring
branch
from
a51a0ec
to
fcedc53
Compare
HansOlsson
approved these changes
Mar 16, 2020
beutlich
force-pushed
the
update-substring
branch
from
123d562
to
089d51d
Compare
casella
reviewed
Jul 8, 2020
There was a problem hiding this comment.
I'm fine with the idea of making this function more tolerant.
I agree with @HansOlsson that you should replace "negative" with "non-positive" or "less than one" in the warning when startIndex < 1
|
@sjoelund is on holiday |
* Do not assert on incorrect arguments of substring, but try to correct them and raise a warning instead. * substring was also improved to return an empty string, in case endIndex is less than startIndex.
beutlich
force-pushed
the
update-substring
branch
from
089d51d
to
8626c64
Compare
That should be the case already. |
casella
approved these changes
Jul 9, 2020
beutlich
added a commit
to beutlich/ModelicaStandardLibrary
that referenced
this pull request
Oct 6, 2020
beutlich
added a commit
that referenced
this pull request
Oct 26, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I missed the feature of Modelica.Utilities.Strings.substring to return an empty string. This is why I started this PR.