Dark Mode

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

PatternRemover Enhancement #1158 and Second Typo #1645#1676

Open
AndyCXL wants to merge 15 commits intowinder:masterfrom
AndyCXL:Replacer
Open

PatternRemover Enhancement #1158 and Second Typo #1645#1676
AndyCXL wants to merge 15 commits intowinder:masterfrom
AndyCXL:Replacer

Conversation

Copy link
Contributor

AndyCXL commented Aug 24, 2021 *
edited
Loading

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
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.

breiler requested changes Aug 26, 2021
AndyCXL added 2 commits August 26, 2021 08:21
.gitignore updated to exclude all-checksum.txt
AndyCXL requested a review from breiler August 26, 2021 07:42
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?

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.

breiler approved these changes Dec 11, 2021
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.

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

Reviewers

breiler breiler approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants