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

PatternRemover Enhancement #1158 and Second Typo #1645 #1676

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

AndyCXL
Copy link
Contributor

@AndyCXL AndyCXL commented Aug 24, 2021

Replacer #1158 (PatternRemover enhancement) and Second Typo #1645 (ABC Step Multiply)

PatternRemover has been enhanced to support 'replace with' and %macro% expansion by introducing 'sed' syntax to the preferences/UGS/Contoller Options.

'sed' syntax is "s/regex to match/replacement text". The code splits the regexpr string passed in (as defined by the user in the Controller Options screen) by "/" and acts according to how many sub-strings are found.

If 1 (and default), ie: no "/", treat this as a simple regex to find a match and replace with "" if found in the gcode.
If 2 then a half-sed syntax was used but the replacement is not specified, so "" is used.
If 3 then first check if the %macro-name% syntax is found in the 'replacement text' string and search the user's macros for a match. If a match is found, the 'replacement text' becomes the 'gcode' content of the macro, if no match, use the replacement text as-is.
It is most commonly expected that the "regex to match" will match and thus replace the whole line, however, note that the "regex to match" may match only a portion of the gcode line, and the replace with functionality only changes the matched text, leaving the remainder of the line unaltered.

The following now all work;

M6T1 in code, "M6T[0-9]+" enabled will REMOVE M6T1 as per original functionality
M6T1 in code, "s/M6T[0-9]+" enabled will REPLACE M6T1 with "" as no replacement text was specified
M6T1 in code, "s/M6T[0-9]+/G21" enabled will REPLACE M6T1 with G21 if that is what the user desires
M6T1 in code, "s/M6T[0-9]+/%1%" enabled will REPLACE M6T1 with whatever macro "1" is defined as by the user in the Gcode column. User must be aware that the result from macro "1" will be passed through as-is.
M6T1 in code, "s/M6T[0-9]+/%Macro #1%" enabled will REPLACE M6T1 with whatever macro "Macro #1" is defined as by the user in the Gcode column. User must be aware that the result from macro "Macro #1" will be passed through as-is.
All REMOVES or REPLACES are logged under INFO to assist user debugging regex and %macro% settings.

Confirmed working (Apple and RPi). Core algorithm re-written to reduce if nesting depths.

Found a typo in jog service.java where getStepSizeZ() was errantly against multiplyABCStepSize(), should have been getStepSizeABC(), which is corrected.

Confirmed working (Apple and RPi).

AndyCXL and others added 12 commits August 4, 2021 14:25
Fixes winder#1645 Menu Multiply XY by 10 issue
Replacer without %macros%
Edge case debugged, no %macro% as yet until backend instance can be obtained (to get to the current Macros() definitions)
Sorted %macro% blockage using SettingsFactory vs Settings
Fixed typo in multiplyABCStepSize(), was using getStepSizeZ() and should have been getStepSizeABC(). Now fixed.
Replacer functionality with %macro% expansion via sed syntax extensively tested and working as intended
More_Typos in JogService.java, fixing divideABCStepSize()
Code Style Tweaks, adding "" to surround log entries to make more legible
Code simplified
@AndyCXL
Copy link
Contributor Author

AndyCXL commented Aug 24, 2021

Over to the moderators to accept, critique or reject this so I know what standard is expected.

lastModified/all-checksum.txt Outdated Show resolved Hide resolved
.gitignore updated to exclude all-checksum.txt
@AndyCXL AndyCXL requested a review from breiler August 26, 2021 07:42
@AndyCXL
Copy link
Contributor Author

AndyCXL commented Dec 9, 2021

This pull has sat untouched for 4 months, having been asked to make a minor change which was done and re-submitted for review. Are contributions like this really wanted, or not really?

@breiler
Copy link
Collaborator

breiler commented Dec 11, 2021

Sorry, it is of course nice with contributions. I do however not feel the ownership of what should be merged and what shouldn't, I feel that it is up to Will. I should have been more clear on where I stood.

This is a nice feature, my only concern is how well this plays together with other features. For instance when using macros there is a feature that can popup asking the user for inputs, what will happen if running this in a headless mode.

I know that this feature is more for expert users, so they would probably know what they are doing. So in that sense this PR looks good to me. I'll leave the merging for Will.

@AndyCXL
Copy link
Contributor Author

AndyCXL commented Jan 11, 2022

I can't tell if the Branch Conflict is actually outstanding or not, or what/if I am required to do to fix it.

@AndyCXL
Copy link
Contributor Author

AndyCXL commented Jul 4, 2022

I give up. Changing to a different sender app now.

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.

2 participants