-
Notifications
You must be signed in to change notification settings - Fork 744
Fix copying First/Last columns with pipe symbols to work in time range input #2304
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
Fix copying First/Last columns with pipe symbols to work in time range input #2304
Conversation
…oard Co-authored-by: brianrob <[email protected]>
Co-authored-by: brianrob <[email protected]>
Co-authored-by: cincuranet <[email protected]>
| // e.g., "| 1,395.251 2,626.358 |" becomes "1,395.251 2,626.358" | ||
| if (!string.IsNullOrEmpty(text)) | ||
| { | ||
| text = text.Trim(' ', '\t', '\n', '\r', '|'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it not be better to include those as range separators and leave it to text.Split? We are lucky here since string is a weird beast, hence we are not actually mutating the incoming text parameter, but in general it feels uneasy when the function parameter is being modified without explicitly being marked by ref or something like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here it is trimming beginning and end. Split would split it everywhere. I guess we could probably after splitting work out only the ones at the start/end of resulting array, but that would be cumbersome. But anyway, feel free to propose better implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh. Missed the part that it would split everywhere. So another approach could be "string index walk" identifying each char and collecting desired indices to be able to cut after a single walk. But I digress, most probably an overkill here.
|
@cincuranet and @tomuxmon thanks for the feedback on this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
![]()
Summary
The fix successfully resolves the issue by adding logic to
RangeUtilities.TryParse()to trim whitespace and pipe symbols from input text before parsing. The optimization reduces the trimming operation from 3 method calls to 1, improving performance while maintaining all functionality. This allows users to copy First/Last columns with the new markdown pipe format and paste them directly into the Start text box, restoring the previous user-friendly behavior while maintaining the new markdown formatting benefits.Original prompt
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.