B2026-022-Adding-RO-Editor-symbols-to-RO-Tables #738

Merged
jjenko merged 5 commits from B2026-022-Adding-RO-Editor-symbols-to-RO-Tables into Development 2026-03-17 16:09:56 -04:00
Owner

B2026-022-Adding-RO-Editor-symbols-to-RO-Tables.

B2026-022-Adding-RO-Editor-symbols-to-RO-Tables.
plarsen self-assigned this 2026-03-16 23:22:11 -04:00
djankowski was assigned by plarsen 2026-03-16 23:22:11 -04:00
jjenko was assigned by plarsen 2026-03-16 23:22:11 -04:00
mvickers was assigned by plarsen 2026-03-16 23:22:11 -04:00
plarsen added 3 commits 2026-03-16 23:22:12 -04:00
plarsen requested review from jjenko 2026-03-16 23:22:56 -04:00
mschill requested changes 2026-03-17 07:52:51 -04:00
mschill left a comment
Owner

See notes - let me know if you have any questions.
Also, this branch needs updated as it is out of date with dev. --- can click the "Update Branch by Merge" button below but then be sure to pull those changes down to your local repo.

See notes - let me know if you have any questions. Also, this branch needs updated as it is out of date with dev. --- can click the "Update Branch by Merge" button below but then be sure to pull those changes down to your local repo.
@@ -2147,6 +2147,7 @@ namespace RODBInterface
foreach (int chr in chrAry)
{
if (chr > 166)
Owner

Should there be a blank line inserted here?

Should there be a blank line inserted here?
Author
Owner

I removed the blank line

I removed the blank line
mschill marked this conversation as resolved
@@ -2204,6 +2205,7 @@ namespace RODBInterface
foreach (int chr in chrAry)
{
if (chr > 166)
Owner

Should there be a blank line inserted here?

Should there be a blank line inserted here?
Author
Owner

I removed the blank line

I removed the blank line
mschill marked this conversation as resolved
@@ -2476,0 +2479,4 @@
foreach (Match match in Regex.Matches(rtn, pattern, RegexOptions.IgnoreCase))
{
mValue = match.Value;
mValue2 = @"\f1 " + mValue + @"\f0";
Owner

Performance - Would avoid adding together strings - would suggest string interpolation.

mValue2 = $"\f1 {mValue}\f0";

note that this viewer changes the double slashes to single ones (unless you click edit on the comment - so see screenshot below)

Performance - Would avoid adding together strings - would suggest string interpolation. mValue2 = $"\\f1 {mValue}\\f0"; note that this viewer changes the double slashes to single ones (unless you click edit on the comment - so see screenshot below)
Author
Owner

I made the change.

I made the change.
mschill marked this conversation as resolved
@@ -2476,0 +2481,4 @@
mValue = match.Value;
mValue2 = @"\f1 " + mValue + @"\f0";
rtn = rtn.Replace(match.Value, mValue2);
// @"\f1\u9474 ?\f0")
Owner

Commented out code should be removed or have text designating why it is not?

Commented out code should be removed or have text designating why it is not?
mschill marked this conversation as resolved
@@ -3827,6 +3840,9 @@ namespace Volian.Controls.Library
this.MergedRanges.Clear();
this.Clear();
this.IsRoTable = true;
//valtext = valtext.Replace(@"\u8209?", "-");
Owner

Commented out code should be removed or have text designating why it is not?

Commented out code should be removed or have text designating why it is not?
Author
Owner

I remove the comment

I remove the comment
mschill marked this conversation as resolved
plarsen added 1 commit 2026-03-17 08:37:31 -04:00
jjenko requested changes 2026-03-17 09:11:28 -04:00
Dismissed
jjenko left a comment
Owner

Please clean up per Matt's comments

Please clean up per Matt's comments
plarsen added 1 commit 2026-03-17 15:29:00 -04:00
mschill requested review from jjenko 2026-03-17 16:05:56 -04:00
mschill requested review from mschill 2026-03-17 16:05:59 -04:00
jjenko approved these changes 2026-03-17 16:09:18 -04:00
jjenko left a comment
Owner

changes look good

changes look good
jjenko merged commit c3cacaf407 into Development 2026-03-17 16:09:56 -04:00
Sign in to join this conversation.
No Reviewers
No Label
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Volian/SourceCode#738