|
|
Post by Stefan on Jan 19, 2021 13:22:30 GMT -5
George,
Really sorry to bring this up, but did your specific fix for this make it into version 2.3.21016?
When MINLEN > 0, a command like CHANGE " " "" 1 ALL still loops/crashes SPFLite.
|
|
|
|
Post by George on Jan 19, 2021 15:29:32 GMT -5
Sigh!
Although the fix is in, and it does handle escaping from totally blank lines, it is basically a miserable failure.
I just traced one and it's pretty sad. Back to the drawing board.
Robert: The problem is as follows:
Picture a line "ABC..." (. are blanks, MINLEN=6)
Change " " "" and CHANGE compares the before/after and determines that, yes, the line has changed (it's now 5 bytes long).
So it stores the line. The line storage guy sees that it is too short, and so applies MINLEN as it is stored (line is back to 6).
Loop-de-loop.
I think a change like this, with MINLEN set should just be rejected as being incompatible, and suggest COMPRESS, or a TR line command.
Unless you guys have some magical logic that can work through this conundrum.
George
|
|
|
|
Post by George on Jan 19, 2021 16:36:33 GMT -5
Not really, the setting of the continue scanning variables is the most sensitive area of what the search routine does. It sets these up and then still has to be able to adjust them based on whether the user has moved the cursor before the RCHANGE occurs. Yes, I know the RCHANGE is internally automatic, but the search routine is unaware of that and just sets everything up "in case".
I truly think this just has to be a rejected, incompatible command. There are just so many alternative commands to accomplish whatever is needed, I can't justify all this 'jumping through hoops' and the risk it entails of mucking something else up.
Yes, if we had some kind of totally thorough test suite that exercised every possible command through every possible combination of user controlled options, maybe we could do such a change and test it. But there is no way on earth such a test suite could even be created. We've got so many options, so many commands etc. that it is logically impossible.
George
|
|
|
|
Post by George on Jan 19, 2021 17:01:45 GMT -5
Robert: That might be an alternative.
Comments?
George
|
|
|
|
Post by Stefan on Jan 19, 2021 18:59:34 GMT -5
I appreciate this is mega-tricky, given the effect MINLEN has on the outcome. I KNOW I can achieve the same result by a variety of other means. But this is not an exercise in avoiding a crash I 'know' will happen (although I'm disappointed with myself for apparently not learning from previous experience!)A single, valid command - whether in common use or not - should not crash the product. If I can crash SPFLite - so can others. And when you have a bunch of open tabs and unsaved data in several of them, it is very annoying when all changes on all tabs are lost because SPFLite disappeared up its own backside - even when you 'knew' but forgot that a simple CHANGE command on an empty line can bring down the whole caboodle. As I see it, there's two ways of tackling this. (1) Fiddle around more with the complex and sensitive code that does the FIND logic. The issue here is that the search code loops over the line repeatedly - without good reason. Consider this (inconsistent) behaviour
CHANGE "A" "AA" ALL 1, SPFLite does not loop, and changes only the 'A's in column 1 to 'AA'. It doesn't fill the line with A's, even though col-1 still contains an A after the first change. CHANGE "A" "A" ALL 1, SPFLite does not loop, and changes only the 'A's in column 1, even if each line reads "AAAAAAA". CHANGE " " " " ALL 1, SPFLite does not loop, and changes ALL blanks in ALL columns (up to MINLEN) when the the line is blank. INCONSISTENT!
CHANGE " " "" ALL 1, SPFlite loops and keeps changing the char in col-1 until is is no longer a blank (and with MINLEN > 0 this never happens on a blank line). Logically, if we change any character in col 1 to something else (including <null>), it is not the original character any more.
So there is no reason to loop around and examine the col-1 char again.
If SPFLite counted <null> as if it had length 1, it would search from col-2 onwards and wouldn't crash cos it would reach the MINLEN limit and move on to the next line.
In any case, if the line to be examined is blank and is being changed to another blank-or-<null>-string, do nothing and move to the next line.
George, I know this is not your favorite approach despite the inconsistent behaviour. I accept that. FIND/CHANGE is one cause of the problem. MINLEN is the other.
UPDATE... As a quick circumvention to avoid the loop/crash, How about you change it so that C "<blanks>" "<null-or-blanks>" is NOT ALLOWED, ever. It would force folks to do CHANGE "banks" "<somthing-unique" followed by CHANGE "<something unique>" "<null>"
(2) Address the whole issue of MINLEN As far as I can see, MINLEN is really helpful to old ISPF/PDF users, because that product ALWAYS treated short lines as having spaces at the end, even for variable length records. So a command like CHANGE '' " "ABCDE" ALL 60 64 placed ABCDE in those columns on ALL lines that to the user 'appear to be' blank in those columns, short lines or not.
SPFLite without MINLEN > 63 does NOT do that.
Robert's MINLEN SAVE approach won't help with the above situation. The padding needs to be there during the edit phase. And I don't want the padded records saving. PRESERVE is OFF even when MINLEN > 0.
WHAT IF
(1) Remove the MINLEN concept altogether AND
(2) amend the FIND code to pad the line with blanks to the length implied by the column range specified in the command, before it is searched.
If the command specifies no column range, no padding is required.
|
|
|
|
Post by Stefan on Jan 20, 2021 4:57:43 GMT -5
Robert, (1) You argue that the 'World is so because that's what CHANGE does'. My point is that the world is how the world is, and CHANGE acts incorrectly in one specific instance. (2) We all agree why it happens (3) Would not fix the CHANGE col 60 64 example. SPFlite still wouldn't do it if the pad were nulls. User would need to issue the command twice, once with <blanks> for lines longer that 64 chars and once with <nulls> for lines shorter that 64 chars. But it's a Possibly acceptable solution(4) A macro would be a user workaround. The change command if issued accidentally still crashes the application.
(5) FIND is not looking for nulls - it's looking for blanks. There is EVERY reason it should find blanks, because MINLEN put them there. The help doc is correct and fine - but it doesn't apply when searching for blanks.
I take your point about NULLS OFF (I had forgotten about that myself :-) ) but MINLEN is SPFLite's implementation of NULLS OFF.
SPFLite has no effective equivalent for NULLS ON, ie "replace trailing blanks with nulls but treat them as blanks when searching or typing something 'behind' them"
(6) Re: FIND "ABC<blanks>" at the end of a line. I take your point.
If 'padding-on-the-fly' were the chosen way forward, this is easily fixed by padding the line with 'n' blanks where 'n' equals the number of chars (or even just trailing blanks) in the <find-str> .
(6a) So that 'CHANGE <blanks> to <something>' works on short and long lines.
(6b) I'm not hoping MINLEN would do anything weird. I just expect it to do what is described - pad the lines with blanks while they are being edited. In that respect, MINLEN is fine. IF RECFM = F is the answer, drop MINLEN entirely and force users to set RECFm to F.
If that were to ensure that all lines loaded were padded and PRESERVE OFF still removes those pads, that would be fine. But I fear the way CHANGE works, it wouldn't fix the issue of the loop/crash.
(6c) Yes, that would work for me. But again, I'm not sure it would fix the loop/crash given the way CHANGE works presently. I don't need the records to be all the SAME length, MINLEN just does it that way. They just need to be padded to some(!) length.
I typically choose 110 bytes (the width of my display), because I avoid lines longer than a display width (never liked scrolling left/right when editing stuff).
Lastly Re: "When MINLEN > 0, the change string in a CHANGE command cannot be of length 0."
I almost completely agree. That's why I added the UPDATE to my previous post this morning.
The loop/crash only occurs when the change command is of the form C "<blanks>" "<null-or-blanks>". So that's the only instance that needs to be disallowed.
A command like CHANGE "<non-blanks>" "<null>" is fine, and extremely useful.
|
|
|
|
Post by George on Jan 20, 2021 9:47:26 GMT -5
Good discussion. From everything here, I'm going with the "When MINLEN > 0, the change string in a CHANGE command cannot be of length 0." approach. Any of the other alternatives are simply too disruptive (code-wise). KISS.
George
|
|
|
|
Post by Stefan on Jan 20, 2021 11:44:31 GMT -5
I thought the loop is caused when the 'search' phase of the CHANGE command repeatedly succeeds in finding a blank < fnd-str> (in whatever column limits) because MINLEN>0 keeps replenishing them after the 'replace' phase has changed them to <null>. So the 'search' phase never reaches the ' no more hits here, go to next line' stage. That can't happen if the < fnd-str> is not a blank string to begin with. A non-blank < fnd-str> will not be found on a 'blank' line (or part thereof), whether truly of zero length or padded with blanks by MINLEN. So the loop doesn't arise because effectively, at the first time of searching, the result is "not found" and the logic will move on to the next line. I believe blocking the more specific (and hence less draconian) C "<blanks>" "<null-or-blanks>" is the better way to go. Otherwise, we're robbed of an effective way to remove a string from a line, e.g. CHANGE "DEF" "" to remove DEF from a line like ABC DEFGH IJKL
Regarding your PS.... The answer is two-fold
1 - Habit. My hands type the command and press <ENTER> before my brain has an opportunity to reason with them. 2 - I'm stupid!
As for COMPRESS
On the upside - it doesn't crash.
But it also doesn't do what I ask it to do.
This is a simple example. In practice the file has hundreds of lines and I need to remove a leading blank from several dozen which occur throughout the file.
CHANGE can do this if it is removing a single <non-blank> character. But crashes when targeting a <blank> for reasons we know.
Neither CHANGE nor COMPRESS can do this correctly - both remove every occurrence of <fnd-str> from the front of the line.
eg: (using + to represent blanks) 00001 +ABC 00002 ++DEF 00003 +++GHI 00004 ++++++++++++++++
00005 JKL
I want to effectively "left-shift" the lines that start with a single blank by 1 column, to remove the '+' in column 1 ONLY.
ie. so I end up with ...
00001 ABC
00002 +DEF
00003 ++GHI
00004 ++++++++++++++++
00005 JKL
COMPRESS "+" "" ALL 1 removes all single and consecutive + characters (fair enough. the single column reference "1" means 'from column-1 onwards')
COMPRESS "+" "" ALL 1 1 removes all single and consecutive '+' characters - tell me how, logically, I can have consecutive characters in a single column? Unsurprisingly, given code reuse, COMPRESS suffers the same repetitive issue as CHANGE, because the <null> shuffles 'new' data into column 1. This data is then found on next search loop looking for more occurrences of <fnd-str> on the same line. Having just changed the data in col 1, that search should have re-started from col-2, and realising that col 2 is outside the specified column limits, should immediately return <not found>.
I know there is a variety of ways of making this happen correctly, from ( line commands to multiple CHANGE commands, to a Macro... etc. But the system will still crash when my (or anyone else's) fingers type faster than their brain reminds them that the application is broken.
|
|
chaat
Sophomore Member
Posts: 59 
|
Post by chaat on Jan 20, 2021 12:35:32 GMT -5
This may be a stupid idea, but would it be possible to only apply the MINLEN logic after the change command has completed instead of doing both the change and the MINLEN in the same looping logic ?
|
|
|
|
Post by Stefan on Jan 21, 2021 7:27:04 GMT -5
Robert,
I've always called it a loop/crash because, technically, SPFLite doesn't crash. It loops endlessly so the user has to click CANCEL the "Loop detected" window. Given the way CHANGE works, the loops would not be endless if MINLEN were 'suspended' in the way you say, but depending on how high the MINLEN value is, there would still be a loop for every 'blank' line. This happens because the line shifts left one char (<null> is nothing) and the CHANGE command re-scans from column 1, so it effectively replaces the next blank there with a <null> 'n' times where 'n' is the MINLEN value. Depending on how many lines there are in the file and how many of them are blank, the 'Loop Detected' window will still appear more often than when MINLEN=0, but at least it wouldn't be an endless loop, so the user can click on <OK> instead of <CANCEL> and avoid losing data from the other open tabs.
I actually like the idea advanced by chaat. In a way, it is similar to what you're proposing in the above post - suspend MINLEN processing until the line is written back. I'm only guessing, butI figurethe code writes the line back every time the CHNAGE logic replaces the <fnd-str> with the <chg-str> because it doesn't know that 'this is the last change to this line' until AFTER the next search for the <fnd-str> fails to find one.
In your reply to chaat you say "George is advancing the cursor by the length of the change string"
That's easily fixed with a statement along the lines of "nextpos = currentpos+MAX(1, length(<chg-str>)"
|
|
|
|
Post by George on Jan 21, 2021 10:47:41 GMT -5
Robert: Stefan: I had the same thought as Stefan overnight, based on his description of the inconsistency in how the resume scan position was set. So, after 2-3 variations of a 1 line change, I think we have a winner. I believe it does things correctly, but knowing how good Stefan is at locating problems, I'll hold my breath till some others have tried it out. Basically it's what Stefan had proposed. sCol += (LEN(lclChg) - 1) ' Adjust continue search col
<< OLD CODE
was changed to sCol += IIF(LEN(lclChg) = 0, 1, LEN(lclChg) - 1) ' Adjust continue search col
<< NEW CODE
If this is OK, then its kind of frustrating to end up with such a simple change. Attached is a test version. SPFLite22.exe (483 KB) George |
|
|
|
Post by George on Jan 21, 2021 12:21:16 GMT -5
Damn!
|
|
|
|
Post by George on Jan 21, 2021 13:36:28 GMT -5
Robert: I think I corrected it, another version attached. The problem is that FIND/CHANGE have to do 2 things, they set the internal default resume scan pointers, and they also have to set the screen cursor position, in case the user doesn't simply hit RFIND/RCHANGE and just moves the cursor somewhere else. When FIND/CHANGE/RFIND/RCHANGE start up, they have to allow the cursor location to override any internal rescan locations. Try it on your X X X X X X X X line. Enter FIND "x", then RFIND, then move the cursor a bit right and hit RFIND. It scans from the cursor location, not the previous RFIND location. Yes, I know a CHANGE ALL never displays the cursor for those 'interim' changes, but logically the cursor HAS been moved. So the fix I put in (which still needed tweaking) had to have a similar tweak to the cursor positioning logic, which it didn't before. As I said before, and why I'm loath to modify this code, it is the most convoluted stuff I've even written. Trying to duplicate ISPF and all its quirks is really tough. Here's the latest incarnation. Watch out, it will have the same version # as my 1st attempt. Make sure the timestamp says 1:21 PM George SPFLite22.exe (483 KB) |
|
|
|
Post by George on Jan 21, 2021 14:57:38 GMT -5
Robert: C'mon, something more than "it doesn't work right".
I just downloaded it and tried your X X X X X X X X test, it worked fine. So did the "Stefan" test for C " " "" 1 ALL.
Just what am I supposed to correct?
George
|
|
|
|
Post by George on Jan 21, 2021 16:00:23 GMT -5
Robert: I again downloaded SPFLite22.EXE. I set MINLEN=30 to match you. C "X" "" ALL works just fine.
Are you sure you're actually running the latest one?
George
|
|
