-
Notifications
You must be signed in to change notification settings - Fork 596
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
#490 - improve code base readability #496
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SikuliX wait_timeout also needs to be changed as part of sikuli_step() when timeout step is used. Otherwise, when present() or visible() is invoked, the SikuliX timeout setting gets reverted back to its internal variable. Also, 'casper' object should be used in displaying the error message instead of 'this'. Otherwise, for the execution context, this would be undefined and error message does not show up.
more details in issue - aisingapore#461
In newer DevTools Protocol, Page.captureScreenshot now supports clip parameter. This allows directly specifying the region to capture for a web element, instead of the previous implementation where a full screen is captured and then clipped as a PhantomJS webpage. This improves performance as only the data of the selector snapshot is needed to be transferred between TagUI and Chrome instead of the whole webpage. Also, as PhantomJS is not involved, the quality and color would be more accurate without additional web profile of PhantomJS. Lastly, this improvement also fixes an issue with snap step in live mode for Chrome, where the snapshot captures full screen instead of only the selector.
this change adds support for escape characters in live mode, for eg when using keyboard step ' will give error and requires creative syntax to type a ' idea for solution is from solving a similar problem during recent development of personal side project - tagui for python
Making a commit to improve on the existing script to kill TagUI processes. For eg if Ctrl+C is used to kill TagUI prematurely, TagUI main process would lose the control to do clean-up of integrations (SikuliX, R, Python, Chrome) and Chrome browser. This shell script (for all the 3 OSes) will kill those processes to free up memory and allows TagUI to start running from a clean state. These actions are not done by default when TagUI is launched, as it is expected that TagUI exits cleanly. Otherwise cleaning up all processes by default will hide potential issues should they arise and they will end up not getting reported.
Follow-up update to previous commit below - Making a commit to improve on the existing script to kill TagUI processes. For eg if Ctrl+C is used to kill TagUI prematurely, TagUI main process would lose the control to do clean-up of integrations (SikuliX, R, Python, Chrome) and Chrome browser. This shell script (for all the 3 OSes) will kill those processes to free up memory and allows TagUI to start running from a clean state. These actions are not done by default when TagUI is launched, as it is expected that TagUI exits cleanly. Otherwise cleaning up all processes by default will hide potential issues should they arise and they will end up not getting reported.
Follow-up update to previous 2 commits below - Making a commit to improve on the existing script to kill TagUI processes. For eg if Ctrl+C is used to kill TagUI prematurely, TagUI main process would lose the control to do clean-up of integrations (SikuliX, R, Python, Chrome) and Chrome browser. This shell script (for all the 3 OSes) will kill those processes to free up memory and allows TagUI to start running from a clean state. These actions are not done by default when TagUI is launched, as it is expected that TagUI exits cleanly. Otherwise cleaning up all processes by default will hide potential issues should they arise and they will end up not getting reported.
Follow-up update to previous 3 commits below by improving to remove Linux warning message - Making a commit to improve on the existing script to kill TagUI processes. For eg if Ctrl+C is used to kill TagUI prematurely, TagUI main process would lose the control to do clean-up of integrations (SikuliX, R, Python, Chrome) and Chrome browser. This shell script (for all the 3 OSes) will kill those processes to free up memory and allows TagUI to start running from a clean state. These actions are not done by default when TagUI is launched, as it is expected that TagUI exits cleanly. Otherwise cleaning up all processes by default will hide potential issues should they arise and they will end up not getting reported.
More details at issue aisingapore#490. Only 7% of files are not formatted for easy reading. It makes no sense to refactor these 7% (3 files tagui_header.js, tagui_parse.php, translate.php) at a risk of hurting existing compatibility and scripts. Solution is to provide beautified snapshots of these 3 files and add link to these snapshots at top of the files, so that any developer who wishes to view or edit the source code can see the beautified version. As TagUI is an engine that tries to translate human languages into working JavaScript code that works with its various components, it has mash-up code using string manipulations and replacements in order to get some things done. Simply using a beautifier may break some parts of the engine that affects some existing users' scripts. Thus it is better to provide beautified versions for the 3% of files that is hard to read, then try to refactor at risk of breaking existing users' scripts.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
More details at issue #490. Only 7% of files are not formatted for easy reading. It makes no sense to refactor these 7% (3 files tagui_header.js, tagui_parse.php, translate.php) at a risk of hurting existing compatibility and scripts. Solution is to provide beautified snapshots of these 3 files and add link to these snapshots at top of the files, so that any developer who wishes to view or edit the source code can see the beautified version.
As TagUI is an engine that tries to translate human languages into working JavaScript code that works with its various components, it has mash-up code using string manipulations and replacements in order to get some things done. Simply using a beautifier may break some parts of the engine that affects some existing users' scripts. Thus it is better to provide beautified versions for the 7% of files that is hard to read, than try to refactor at risk of breaking existing users' scripts.