|
|
Post by Stefan on Feb 28, 2022 7:22:40 GMT -5
George,
TAG works a bit differenctly when compared to the other commands like FIND, CHANGE, etc.
This may not be a bug as such, but the documentation is confusing. In the HELP Syntax box for TAG, the underlined default is shown as ALL.
But in the description below for FIRST, LAST, NEXT, PREV and ALL, the NEXT value is underlined as default.
In line with similar commands, I expected that the command TAG :XX ON "keyword" will default to NEXT and therefore tag the next occurence. In fact, that command tags ALL occurences.
In any case, whether you specify NEXT or ALL on the TAG command, makes no difference.
The command TAG :XX ON "keyword" NEXT will also tag all occurences.
That is definitely wrong.
Also, is there a reason why TAG takes ALL as default when all other commands use NEXT?
|
|
|
|
Post by George on Mar 1, 2022 9:40:17 GMT -5
Robert: I agree with you, TAG simply IS differeeent. But perhaps, if they're not effective, the FIRST/LAST/NEXT/ALL should be removed as options and the Doc updated.
|
|
|
|
Post by Stefan on Mar 1, 2022 9:59:36 GMT -5
Robert, I also agree with you, thakx for clearing that up.
However, as a user, I don't have your insight into the 'intent' behind a particular feature. I can only judge by what I read in the documentation. And TAG doesn't do what the documentation says. I raised a bug because more often that not, it is the code that deviates from the documented intent. In this case, the code is fine and the documentation needs tweaking. No drama. Just move this thread to the documentation section and all is well.
George,
I was further confused since TAG seems to honour PREV, FIRST and LAST, just not NEXT.
But that's irrelevant if you pull all that syntax from the command.
|
|
|
|
Post by George on Mar 1, 2022 11:28:49 GMT -5
OK, I thought the report was that ALL was assumed regardless of the others. I'll have a look at the code, knowing how search works, I'm surprised NEXT is being ignored.
George
|
|
|
|
Post by George on Mar 1, 2022 11:40:56 GMT -5
OK, basically PREV / NEXT are not supported in TAG (not even sure how it WOULD work, I'm not even going to try), so I'm going to simply reject them.
TAG parsing is really convoluted, even after calling the generalized parser there's lots of 'wrinkles' to be done.
So say goodbye to PREV / NEXT.
George
|
|
|
|
Post by George on Mar 2, 2022 9:42:18 GMT -5
Well, a surprise as I went to remove PREV / NEXT from TAG, since they weren't supported.
1. They are supported and work just fine.
2. The problem was that the NEXT keyword has been 'broken' in the parser for quite a while. But because NEXT is the default for so many commands, it is rarely actually entered. The error looks like a simple typo error, so I corrected that and TAG PREV/NEXT now work as designed.
Never underestimate the power of a finger-fumble.
George
|
|
|
|
Post by George on Mar 2, 2022 11:46:09 GMT -5
Robert: I don't disagree with what you said, but NEXT was NOT working. My thought, since the TAG command is a rather convoluted command, was that if PREV/NEXT aren't working, and the default ALL was the normal usage, then why waste time trying to debug WHY some almost unused operands aren't working? Just reject them if coded. I was NEVER proposing to remove a working function. So I added 2 lines of code to reject them (e.g IF PREV OR IF NEXT THEN issue MSG). And it worked properly for PREV and had no effect on NEXT.  ?? After a ridiculous amount of debugging I discovered that NEXT was not being handled properly by the main parser, (which was the LAST place I expected to find it) and was ineffective for ALL commands, NEXT was basically invisible. The cause was a plain, dumb typo, just not obvious (see below - can you spot it?). But it does show how infrequently users ever code NEXT, since the bug has been there for quite a few releases. Stefan only just uncovered it. George SELECT CASE AS LONG gCmdOpsType(i) ' Split by KW
CASE %KWTOP: IF ALLOK THEN CRT.FlgTop = %True: ITERATE FOR
CASE %KWALL: IF ALLOK THEN CRT.FlgAll = %True: ITERATE FOR
CASE %KWEXCLUDE: IF SubSet THEN CRT.FlgX = %True: ITERATE FOR
CASE %KWNX: IF SubSet THEN CRT.FlgNX = %True: ITERATE FOR
CASE %KWFIRST: IF Direct THEN CRT.FlgFirst = %True: ITERATE FOR
CASE %KWLAST: IF Direct THEN CRT.FlgLast = %True: ITERATE FOR
CASE %KWPREV: IF Direct THEN CRT.FlgPrev = %True: ITERATE FOR
CASE %KWNEXT: IF Direct THEN CRT.FlgNext: ITERATE FOR
CASE %KWWORD: IF Modifier THEN CRT.FlgWord = %True: ITERATE FOR
CASE %KWCHARS: IF Modifier THEN CRT.FlgChars = %True: ITERATE FOR
CASE %KWPREFIX: IF Modifier THEN CRT.FlgPrefix = %True: ITERATE FOR
CASE %KWSUFFIX: IF Modifier THEN CRT.FlgSuffix = %True: ITERATE FOR
CASE %KWLM: IF Modifier THEN CRT.FlgLM = %True: ITERATE FOR
CASE %KWRM: IF Modifier THEN CRT.FlgRM = %True: ITERATE FOR
CASE %KWMX: IF Exclude THEN CRT.FlgMX = %True: ITERATE FOR
CASE %KWDX: IF Exclude THEN CRT.FlgDX = %True: ITERATE FOR
CASE %KWCS: IF CShift THEN CRT.FlgCS = %True: ITERATE FOR
CASE %KWDS: IF CShift THEN CRT.FlgDS = %True: ITERATE FOR
CASE %KWLEFT: IF LR THEN CRT.FlgLeft = %True: ITERATE FOR
CASE %KWRIGHT: IF LR THEN CRT.FlgRight = %True: ITERATE FOR
CASE %KWNF: IF Negative THEN CRT.FlgNF = %True: ITERATE FOR
CASE %KWON: IF lclTag THEN CRT.FlgON = %True: ITERATE FOR
CASE %KWOFF: IF lclTag THEN CRT.FlgOff = %True: ITERATE FOR
CASE %KWTOGGLE: IF lclTag THEN CRT.FlgToggle = %True: ITERATE FOR
CASE %KWASSERT: IF lclTag THEN CRT.FlgAssert = %True: ITERATE FOR
CASE %KWSET: IF lclTag THEN CRT.FlgSet = %True: ITERATE FOR
CASE %KWTRUNC: IF L2 THEN CRT.FlgL2TRUNC = %True: ITERATE FOR
CASE %KWSTD: IF Pen THEN CRT.FlgStd = %True: ITERATE FOR
CASE %KWMSTD: IF Pen THEN CRT.FlgMStd = %True: ITERATE FOR
CASE %KWPSTD: IF Pen THEN CRT.FlgPStd = %True: ITERATE FOR
CASE %KWSOLID: IF Pen THEN CRT.FlgSolid = %True: ITERATE FOR
CASE %KWMSOLID: IF Pen THEN CRT.FlgMSolid = %True: ITERATE FOR
CASE %KWU: IF UserL THEN CRT.FlgU = %True: ITERATE FOR
CASE %KWNU: IF UserL THEN CRT.FlgNU = %True: ITERATE FOR
CASE %KWMAX: IF LMax THEN CRT.FlgMax = %True: ITERATE FOR
CASE ELSE
tMsg = "Keyword not allowed on the " + gCmdOps(0) + " command - " + gCmdOps(1): GOTO CRTBailOut
END SELECT '
|
|
|
|
Post by George on Mar 2, 2022 15:33:06 GMT -5
Robert: It took WAY to long to find because I kept saying "Well parse MUST be correct or we'd have a whole bunch of other reported problems". Wouldn't you know the error was on the one keyword which is probably the least used because it is nearly always the default.
And the way parse is coded with lots of KW equates, which end up as single byte CHR$ values (because of the heavy use of INSTR$ in the validation) it is very hard to easily 'watch' the data in Debug mode. Everything ganged up at once to make debugging more difficult.
I'm sure there's lots of other 'lurking' bugs in there, finger-fumbles happen all the time.
George
|
|
??