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

Convert Post Processing Config to VueRouter #4946

Merged
merged 14 commits into from
Sep 5, 2018

Conversation

sharkykh
Copy link
Contributor

@sharkykh sharkykh commented Aug 23, 2018

  • Add missing config.vue render test
  • Convert /config/postProcessing to SFC
  • Add render test for config-post-processing.vue

@sharkykh sharkykh added this to the 0.2.9 milestone Aug 23, 2018
@sharkykh sharkykh changed the title Vueify/config post processing sfc Convert /config/postProcessing to SFC Aug 23, 2018
@OmgImAlexis OmgImAlexis self-requested a review August 23, 2018 09:20
@sharkykh sharkykh changed the title Convert /config/postProcessing to SFC Convert Post Processing Config to VueRouter Aug 23, 2018
@sharkykh sharkykh added the Changelog Requires a changelog entry label Aug 23, 2018
@sharkykh sharkykh force-pushed the vueify/config-post-processing-sfc branch 2 times, most recently from 5033344 to 9152e2c Compare August 23, 2018 18:19
@sharkykh sharkykh added Concluded Needs testing Requires testing to make sure it's working as intended and removed Changelog Requires a changelog entry In progress labels Aug 23, 2018
@sharkykh
Copy link
Contributor Author

sharkykh commented Aug 23, 2018

Still need to test this a bit more to make sure I didn't break anything, but it looks good so far.
In any case, this is ready for a review.

@sharkykh sharkykh force-pushed the vueify/config-post-processing-sfc branch from 8220eb8 to cefe4bf Compare August 29, 2018 13:38
Database:␊
<span>44.11</span></td></tr><tr><td><i class="icon16-config-python"></i> Python Version:</td><td></td></tr><tr><td><i class="icon16-config-ssl"></i> SSL Version:</td><td></td></tr><tr><td><i class="icon16-config-os"></i> OS:</td><td></td></tr><tr><td><i class="icon16-config-locale"></i> Locale:</td><td></td></tr><tr><td>&nbsp;</td><td>&nbsp;</td></tr><tr class="infoTableSeperator"><td>&nbsp;</td><td>&nbsp;</td></tr><tr><td><i class="icon16-config-user"></i> User:</td><td></td></tr><tr><td><i class="icon16-config-dir"></i> Program Folder:</td><td></td></tr><tr><td><i class="icon16-config-config"></i> Config File:</td><td></td></tr><tr><td><i class="icon16-config-db"></i> Database File:</td><td></td></tr><tr><td><i class="icon16-config-cache"></i> Cache Folder:</td><td></td></tr><tr><td><i class="icon16-config-log"></i> Log Folder:</td><td></td></tr><tr><td><i class="icon16-config-arguments"></i> Arguments:</td><td><pre>[␊
"--nolaunch",␊
"--datadir=C:\\\\Medusa\\\\Data"␊
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unsure if it's related to this PR but what's up with the 4 backslashes?

Copy link
Contributor Author

@sharkykh sharkykh Sep 2, 2018

Choose a reason for hiding this comment

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

I copied the values from the Vuex tab on devtools, it doubles everything. I'll fix that.

Copy link
Contributor Author

@sharkykh sharkykh Sep 2, 2018

Choose a reason for hiding this comment

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

Actually, looks like the original value is correct.
It might be the prettyPrintJSON method...

@sharkykh
Copy link
Contributor Author

sharkykh commented Sep 4, 2018

This has been open for a while now.
If this could get included in 0.2.9 it would be best, as it is essentially a follow-up PR for #4259 + #4872

Database:␊
<span>44.11</span></td></tr><tr><td><i class="icon16-config-python"></i> Python Version:</td><td>2.7.15 (v2.7.15:ca079a3ea3, Apr 30 2018, 16:30:26) [MSC v.1500 64 bit (AMD64)]</td></tr><tr><td><i class="icon16-config-ssl"></i> SSL Version:</td><td></td></tr><tr><td><i class="icon16-config-os"></i> OS:</td><td></td></tr><tr><td><i class="icon16-config-locale"></i> Locale:</td><td></td></tr><tr><td>&nbsp;</td><td>&nbsp;</td></tr><tr class="infoTableSeperator"><td>&nbsp;</td><td>&nbsp;</td></tr><tr><td><i class="icon16-config-user"></i> User:</td><td></td></tr><tr><td><i class="icon16-config-dir"></i> Program Folder:</td><td></td></tr><tr><td><i class="icon16-config-config"></i> Config File:</td><td></td></tr><tr><td><i class="icon16-config-db"></i> Database File:</td><td></td></tr><tr><td><i class="icon16-config-cache"></i> Cache Folder:</td><td></td></tr><tr><td><i class="icon16-config-log"></i> Log Folder:</td><td></td></tr><tr><td><i class="icon16-config-arguments"></i> Arguments:</td><td><pre>[␊
"--nolaunch",␊
"--datadir=C:\\\\Medusa\\\\Data"␊
Copy link
Collaborator

Choose a reason for hiding this comment

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

@sharkykh didn't you fix this?

Copy link
Contributor Author

@sharkykh sharkykh Sep 6, 2018

Choose a reason for hiding this comment

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

I did not. The "problem" here is the response from the API is correct (C:\Medusa\Data), then when applying JSON.stringify on it, it escapes the backslashes and each \ becomes \\ and that is what is printed. The snapshot simply escapes it again, and that's why it's 4 backslashes for each original backslash.
I believe the original Python version of this page used json.dumps which would produce the same result, so I figured it's correct. 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

No worries, just wanted to make sure in the case you had fixed it that it hadn't snuck back in.

Copy link
Collaborator

@OmgImAlexis OmgImAlexis left a comment

Choose a reason for hiding this comment

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

LGTM

@OmgImAlexis OmgImAlexis merged commit 7d82f9e into develop Sep 5, 2018
@OmgImAlexis OmgImAlexis deleted the vueify/config-post-processing-sfc branch September 5, 2018 23:49
@OmgImAlexis OmgImAlexis removed Needs review Needs testing Requires testing to make sure it's working as intended labels Sep 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants