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

Disabling JS code insertion due JSRefactoring into PHP document or in any non-js scope #14603

Open
wants to merge 6 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@gautam0217
Copy link

gautam0217 commented Nov 29, 2018

JSRefactoring was inserting JS code into PHP document while using option wrap in condition/try-catch.

js-refactoring_in_php

Added check to make wrap in condition/try-catch in non-js context as no-op.

@gautam0217

This comment has been minimized.

Copy link

gautam0217 commented Nov 29, 2018

closing now and will reopen later, so that Travis-CI build will be triggered again.
travis-ci/travis-ci#576 (comment)

@gautam0217 gautam0217 closed this Nov 29, 2018

@gautam0217 gautam0217 reopened this Nov 29, 2018

@navch

This comment has been minimized.

Copy link
Collaborator

navch commented Nov 30, 2018

Thanks @gautam0217 for this PR. But I guess disabling menu in all documents (except js) will be a better solution.

@navch

This comment has been minimized.

Copy link
Collaborator

navch commented Nov 30, 2018

It looks like cla is not signed that's why build is failing. and you can also trigger build after logging in there.

@gautam0217

This comment has been minimized.

Copy link

gautam0217 commented Nov 30, 2018

As we support mixed mode(e.g. HTML+JS etc.) in refactoring, making operations no-op is more aligned to current approach.

@navch

This comment has been minimized.

Copy link
Collaborator

navch commented Nov 30, 2018

Yep got that. I am saying disable the menu instead of just returning from there otherwise the user will use that menu and he/she will be got confused (i.e. someone pressed menu and nothing happened either popup some error or disable the menu will be better solution, you can use this "displayErrorMessageAtCursor" to show error .)

@gautam0217

This comment has been minimized.

Copy link

gautam0217 commented Nov 30, 2018

Added generic error message for each case like following:

try-catch-err-msg

@navch

This comment has been minimized.

Copy link
Collaborator

navch commented Dec 3, 2018

Is it possible to add a generic error like As of now Refactoring is available only for JS?

@gautam0217

This comment has been minimized.

Copy link

gautam0217 commented Dec 4, 2018

Thanks @gautam0217 for this PR. But I guess disabling menu in all documents (except js) will be a better solution.

Yes, I think we should ignore this PR and work on disabling JS refactor menu in non-JS document.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment