T1.5-Modify the VisualDifferenceViewer so text wraps instead of using scrollviewers #10932

SHKnudsen · 2020-07-23T16:29:50Z

Based on PR #10928 , please merge that first

Purpose

This PR addresses the wish to have the text in the Code Difference viewer wrap instead of using scrollviewers, to make it more in style with the Python script editor. I have looked into modifying the DiffPlex viewer but couldn't find a way to do this. This PR creates new views that behaves and looks like the original DiffPlex viewer but adds wrapping to the text.

The caveat of this approach is that it adds a lot more code than using the DiffPlex viewer. We still need the Dependence on DiffPlex but we can remove the reference to DiffPlex.Wpf

Declarations

Check these if you believe they are true

The codebase is in a better state after this PR
Is documented according to the standards
The level of testing this PR includes is appropriate
User facing strings, if any, are extracted into *.resx files
All tests pass using the self-service CI.
Snapshot of UI changes, if any.
Changes to the API follow Semantic Versioning and are documented in the API Changes document.

Reviewers

@mmisol

FYIs

@mjkkirschner
@QilongTang
@Amoursol
@radumg

* add ScriptMigrator * Add visual difference viewer * Update PythonMigrationViewExtension.cs * Add tooltip description to migration assistant * updates * comment updates * comment updates

* add ScriptMigrator * Add visual difference viewer * Added methods for updating the ScriptEditors with the migrated code * Update PythonMigrationViewExtension.cs * Add tooltip description to migration assistant * updates * comment updates * comment updates * Update ScriptEditorWindow.xaml.cs * Update PythonMigrationViewExtension.csproj * Update PythonMigrationViewExtension.csproj * comment updates * Revert "comment updates" This reverts commit 2d2332a. * comment updates * Update PythonMigrationAssistantViewModel.cs * comment updates 2

* add ScriptMigrator * Add visual difference viewer * Added methods for updating the ScriptEditors with the migrated code * Update PythonMigrationViewExtension.cs * add mechanisme to save Python 2 backup * Add tooltip description to migration assistant * updates * comment updates * comment updates * Update ScriptEditorWindow.xaml.cs * Update PythonMigrationViewExtension.csproj * Update PythonMigrationViewExtension.csproj * comment updates * Revert "comment updates" This reverts commit 2d2332a. * comment updates * Update PythonMigrationAssistantViewModel.cs * comment updates 2 * small clean ups * Update PythonMigrationAssistantViewModel.cs

…dsen/Dynamo into T1.3-SavePython2BackupFile

…kupFile

…ionAssistantTests

…kupFile

…-CodeChanges

…github.com/SHKnudsen/Dynamo into pr/10932

radumg · 2020-07-29T20:32:42Z

To solve conflicts ahead of time and incorporate fixes, i've rebased this on the branch for #10928 , so please merge that first and then this one.

radumg · 2020-07-29T21:12:33Z

The width of the code (text) area can't be matched when window size is exactly the same as we're always going to have the additional space for the + or - indicators, so went ahead with subtly making the differ windows larger.

Also used the lorem ipsum text as a more accurate indicator on how the word wrap affects multi-line paragraphs.

Inline

Side by side

also allow for slightly larger vertical height for Before/After label as it was hard to spot

radumg · 2020-07-29T22:00:24Z

Colours updated

update red/green to suit supplied colours
updated vertical splitter in side comparison layout to be black instead of white to better match the Dynamo viewports

radumg · 2020-07-30T00:10:26Z

Also upgraded the inline view to now show the exact fragments of what's changed, instead of a simple deleted/changed for the entire line.

Before

Entire block of text is highlighted the same way.

After

Block of text has background with change type
Actual changed bits, the ( and ) are highlighted

radumg · 2020-07-30T00:15:30Z

That should wrap this one up as well, please have a look @mmisol , @mjkkirschner

Amoursol · 2020-07-30T13:38:34Z

Looking pretty epic to me @radumg! Thank you.

mmisol · 2020-07-31T15:44:15Z

@Amoursol Bringing this to your attention because it got hidden in a conversation #10932 (review)

To summarize, I can see a slight difference in wrapping that @radumg was not able to reproduce on his end. It could be related to a combination of DPI settings and screen resolution but we are not sure. How do you think we should deal with this?

Amoursol · 2020-07-31T20:12:05Z

@mmisol Unless we can reproduce the issue, in which case we should definitely resolve, I am fine with minor visual differences. Do we have a 3rd machine we can test on? One of our Virtual test machines? It may be a Mac vs. Windows issue.

SHKnudsen · 2020-08-03T11:55:05Z

@mmisol @radumg tested this on my machine as well and everything seems to wrap as expected. I made a few small changes to make sure the sizing of the diff viewer is consistent.

mmisol · 2020-08-03T12:55:32Z

Thanks for testing @SHKnudsen . Still wraps different for me but this should be good to go. I'll delay approval until we merge the other PR, just in case to prevent any issues.

radumg · 2020-08-03T13:22:02Z

src/PythonMigrationViewExtension/Controls/BaseDiffViewer.xaml.cs

@@ -19,6 +19,8 @@ internal BaseDiffViewer(PythonMigrationAssistantViewModel viewModel)
        {
            ViewModel = viewModel;
            DataContext = viewModel;
+            // The diff viewer is initialized with the SideBySide view
+            SetSideBySideWindowWidth();


@SHKnudsen , let's put this after the initialize component method call

mmisol · 2020-08-04T18:22:27Z

@SHKnudsen @radumg #10928 got merged. Could you rebase this PR so that we can approve it and merge it too?

SHKnudsen · 2020-08-05T09:57:16Z

@mmisol should be ready now.

mmisol

Thanks @SHKnudsen . Tests worked fine here https://master-5.jenkins.autodesk.com/job/Dynamo/job/DynamoSelfServe/job/pullRequestValidation/1201/. I'll proceed and merge.

SHKnudsen added 30 commits July 3, 2020 08:19

T1.1 Code Migration (#4)

9e29adc

* add ScriptMigrator * Add visual difference viewer * Update PythonMigrationViewExtension.cs * Add tooltip description to migration assistant * updates * comment updates * comment updates

Update PythonMigrationViewExtension.csproj

0fbd696

update engine version when migrating code

1684a49

Merge remote-tracking branch 'upstream/master' into T1.1-CodeMigration

e52d617

extension and test updates

b7fe228

Merge branch 'T1.1-CodeMigration' into T1.2-AcceptReject-CodeChanges

f168751

Merge branch 'T1.3-SavePython2BackupFile' of https://github.com/SHKnu…

7229746

…dsen/Dynamo into T1.3-SavePython2BackupFile

add 2to3 icon

6caa0db

Merge branch 'T1.1-CodeMigration' into T1.2-AcceptReject-CodeChanges

451a980

Merge branch 'T1.2-AcceptReject-CodeChanges' into T1.3-SavePython2Bac…

9542bf8

…kupFile

comment updates

8a70055

GraphPythonDependencies changes

f9ab1d5

Merge branch 'master' into T1.1-CodeMigration

8d2f36b

make event internal

f3edb0f

fix build failure

3f264dd

update license files, MigrationAssitantClicked event and PythonMigrat…

c34f729

…ionAssistantTests

Merge branch 'master' into T1.1-CodeMigration

fdf7fe6

Merge branch 'T1.1-CodeMigration' into T1.2-AcceptReject-CodeChanges

904dee5

clean up

d497eb4

Merge branch 'T1.2-AcceptReject-CodeChanges' into T1.3-SavePython2Bac…

33f5aad

…kupFile

clean up

235851d

update licence files

cfb0a98

Merge branch 'T1.1-CodeMigration' into T1.2-AcceptReject-CodeChanges

6912267

Merge branch 'T1.2-AcceptReject-CodeChanges' into T1.3-SavePython2Bac…

acbe079

…kupFile

Merge remote-tracking branch 'upstream/master' into T1.2-AcceptReject…

9bec448

…-CodeChanges

make MigrateCode internal

f06d7a2

comments update

088e008

unsubscribe from ScriptEditorWindow closed

303c644

radumg added 3 commits July 29, 2020 22:19

parametrise the differ window initial width

0e54cac

resolve conflict between the 2 python migration viewmodels

d260338

Merge branch 'T1.5-VisualDifferenceViewer-With-text-wrap' of https://…

4c97566

…github.com/SHKnudsen/Dynamo into pr/10932

radumg added the DNM Do not merge. For PRs. label Jul 29, 2020

radumg added 2 commits July 29, 2020 23:37

adjust initial window widths to keep text same wrapping

1bceea7

also allow for slightly larger vertical height for Before/After label as it was hard to spot

update diff colours to Dynamo pallette

425a7bd

radumg added 2 commits July 30, 2020 01:37

update splitter colour in side diff layout to be black

12ee797

upgrade the inline display mode to also show fragments

75a60bc

radumg added Python UI and removed DNM Do not merge. For PRs. labels Jul 30, 2020

radumg removed their request for review July 30, 2020 00:14

update sizing of diff viewer

400749e

radumg reviewed Aug 3, 2020

View reviewed changes

SHKnudsen added 2 commits August 5, 2020 10:50

Merge branch 'master' into T1.5-VisualDifferenceViewer-With-text-wrap

1926384

Update PythonMigrationViewExtensionTests.cs

32dc7f0

mmisol approved these changes Aug 5, 2020

View reviewed changes

mmisol merged commit 6fe9df4 into DynamoDS:master Aug 5, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

T1.5-Modify the VisualDifferenceViewer so text wraps instead of using scrollviewers #10932

T1.5-Modify the VisualDifferenceViewer so text wraps instead of using scrollviewers #10932

SHKnudsen commented Jul 23, 2020 •

edited by radumg

Loading

radumg commented Jul 29, 2020

radumg commented Jul 29, 2020 •

edited

Loading

radumg commented Jul 29, 2020 •

edited

Loading

radumg commented Jul 30, 2020 •

edited

Loading

radumg commented Jul 30, 2020

Amoursol commented Jul 30, 2020

mmisol commented Jul 31, 2020

Amoursol commented Jul 31, 2020

SHKnudsen commented Aug 3, 2020

mmisol commented Aug 3, 2020

radumg Aug 3, 2020

mmisol commented Aug 4, 2020

SHKnudsen commented Aug 5, 2020

mmisol left a comment

T1.5-Modify the VisualDifferenceViewer so text wraps instead of using scrollviewers #10932

T1.5-Modify the VisualDifferenceViewer so text wraps instead of using scrollviewers #10932

Conversation

SHKnudsen commented Jul 23, 2020 • edited by radumg Loading

Based on PR #10928 , please merge that first

Purpose

Declarations

Reviewers

FYIs

radumg commented Jul 29, 2020

radumg commented Jul 29, 2020 • edited Loading

Inline

Side by side

radumg commented Jul 29, 2020 • edited Loading

radumg commented Jul 30, 2020 • edited Loading

Before

After

radumg commented Jul 30, 2020

Amoursol commented Jul 30, 2020

mmisol commented Jul 31, 2020

Amoursol commented Jul 31, 2020

SHKnudsen commented Aug 3, 2020

mmisol commented Aug 3, 2020

radumg Aug 3, 2020

Choose a reason for hiding this comment

mmisol commented Aug 4, 2020

SHKnudsen commented Aug 5, 2020

mmisol left a comment

Choose a reason for hiding this comment

SHKnudsen commented Jul 23, 2020 •

edited by radumg

Loading

radumg commented Jul 29, 2020 •

edited

Loading

radumg commented Jul 29, 2020 •

edited

Loading

radumg commented Jul 30, 2020 •

edited

Loading