Skip to content
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

DYN-6794 python script editor convert legacy tabs to spaces #15179

Conversation

ivaylo-matov
Copy link
Contributor

Purpose

PR to address https://jira.autodesk.com/browse/DYN-6794 & #15033 .
Added button in the Python script editor to convert tab indentations from legacy Python code to space indentations.
Includes test.

DYN-6794-fix

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.
  • This PR modifies some build requirements and the readme is updated
  • This PR contains no files larger than 50 MB

Release Notes

Added button in Python to convert tab indentations to space indentations so that users can run legacy code.
Icon for the convert button is a place holder.

PS: Tried a few options where we suppress the conversion to spaces in PythonIndentationStrategy but ultimately they all lead to problems beyond just pasting code with tab indentations. Preserving the current strategy and converting legacy code to the modern convention seem a better solution.

Reviewers

@dnenov
@reddyashish

FYIs

@Amoursol

- button to convert legacy tabs to space indents
- holds pace indentation strategy even in legacy code
- test
- clean
- button icon is just a placeholder
@avidit avidit changed the title Dyn 6794 python script editor convert legacy tabs to spaces DYN-6794 python script editor convert legacy tabs to spaces Apr 29, 2024
@reddyashish reddyashish marked this pull request as ready for review April 29, 2024 19:33
@QilongTang QilongTang added this to the 3.2 milestone Apr 30, 2024
Copy link

github-actions bot commented Apr 30, 2024

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net8.0

Copy link
Contributor

@QilongTang QilongTang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a few comments

@QilongTang
Copy link
Contributor

Waiting for PR checks before merging

Copy link
Contributor

@reddyashish reddyashish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running the job again.

@reddyashish
Copy link
Contributor

https://master-5.jenkins.autodesk.com/job/Dynamo/job/DynamoSelfServe/job/pullRequestValidation/15991/testReport/ looks good. Only 1 test GraphNodeManagerViewExtensionTests.ContainsEmptyListOrNullTest has failed which was recently changed to fix the flaky behavior.

@reddyashish reddyashish merged commit 574e805 into DynamoDS:master May 6, 2024
22 of 23 checks passed
@chuongmep
Copy link
Contributor

chuongmep commented May 7, 2024

Can I know what happen with the prevous script ? So it will automation or show the issue when user keep use old dynamo python script ? I don't want to see the same headache as Ironpython 2.7 converts to Cpython3, causing the sets to break.

@QilongTang
Copy link
Contributor

Can I know what happen with the prevous script ? So it will automation or show the issue when user keep use old dynamo python script ? I don't want to see the same headache as Ironpython 2.7 converts to Cpython3, causing the sets to break.

Hi @chuongmep My understanding is that there is no automation here, so if you open a legacy script, it should not be affected without user manually clicking on that new convert button.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants