Re: [Lazarus] Masks: the naming of ...
27.10.2021 18:50, Bart via lazarus пишет: Hi, I thought I better start a new therad for this one, otherwise I get lost in the previous "TMask revisited" thread. I would like to rename some stuff, now we still can. Easier to remeber IMO: WindowsQuirksAllAllowed -> AllWindowsQuirks WindowsQuirksDefaultAllowed -> DefaultWindowsQuirks MaskOpCodesAllAllowed -> AllMaskOpcodes MaskOpCodesDefaultAllowed -> DefaultMaskOpcodes property RangeAutoReverse -> AutoReverseRange Internal consistency: TMaskWindows -> TWindowsMask TMaskListWindows -> TWindowsMaskList (Because of the Matches* functions we already have) Consitent coding style: TMaskParsedCode enums: mpcXXX TMaskFailCause enums: mfcXXX Opinions please. Looks good to me. -- Best regards, Maxim Ganetsky mailto:gan...@narod.ru -- ___ lazarus mailing list lazarus@lists.lazarus-ide.org https://lists.lazarus-ide.org/listinfo/lazarus
Re: [Lazarus] Masks: ConstructLegacy
28.10.2021 0:33, Bart via lazarus пишет: On Wed, Oct 27, 2021 at 11:17 PM Juha Manninen via lazarus wrote: Attached the codetools popup for TMask.Create constructor. I would think it would be clear enough? It is clear for people who know the details already. For new users there is no hint of an extended syntax. Anyway, we can consider it as an advanced feature which requires users to study deeper. No problem. I'm OK with leaving them in, but in time they should be removed. CreateLegacy in version 3.6 is going to look a little bit "off". We want people staring to use the "new" syntax (that is: use the additional last parameter(s)) as fast as possible. Maybe deprecate them in 2.5 and remove in whatever we release after 2.4? I don't see the need in CreateLegacy/CreateExtended constructors as far as backward compatibility is maintained (it is the case as far as I understand). Just commit your Create constructors refactoring and document the changes (and update release notes). -- Best regards, Maxim Ganetsky mailto:gan...@narod.ru -- ___ lazarus mailing list lazarus@lists.lazarus-ide.org https://lists.lazarus-ide.org/listinfo/lazarus
Re: [Lazarus] Masks: ConstructLegacy
28.10.2021 0:17, Juha Manninen via lazarus пишет: On Thu, Oct 28, 2021 at 12:02 AM Bart via lazarus mailto:lazarus@lists.lazarus-ide.org>> wrote: Attached the codetools popup for TMask.Create constructor. I would think it would be clear enough? Ok, if you say so. :) It is clear for people who know the details already. For new users there is no hint of an extended syntax. I like Bart's proposal. This syntax is extended only for now, in a year it will be perceived as pretty much standard, so such constructor division (Create/CreateExtended/CreateLegacy) will itself be considered a quirk. ;) Anyway, we can consider it as an advanced feature which requires users to study deeper. No problem. -- Best regards, Maxim Ganetsky mailto:gan...@narod.ru -- ___ lazarus mailing list lazarus@lists.lazarus-ide.org https://lists.lazarus-ide.org/listinfo/lazarus
Re: [Lazarus] Masks: ConstructLegacy
On Wed, Oct 27, 2021 at 11:17 PM Juha Manninen via lazarus wrote: >> Attached the codetools popup for TMask.Create constructor. >> I would think it would be clear enough? > It is clear for people who know the details already. For new users there is > no hint of an extended syntax. > Anyway, we can consider it as an advanced feature which requires users to > study deeper. No problem. I'm OK with leaving them in, but in time they should be removed. CreateLegacy in version 3.6 is going to look a little bit "off". We want people staring to use the "new" syntax (that is: use the additional last parameter(s)) as fast as possible. Maybe deprecate them in 2.5 and remove in whatever we release after 2.4? -- Bart -- ___ lazarus mailing list lazarus@lists.lazarus-ide.org https://lists.lazarus-ide.org/listinfo/lazarus
Re: [Lazarus] Masks: ConstructLegacy
On Thu, Oct 28, 2021 at 12:02 AM Bart via lazarus < lazarus@lists.lazarus-ide.org> wrote: > Attached the codetools popup for TMask.Create constructor. > I would think it would be clear enough? > Ok, if you say so. :) It is clear for people who know the details already. For new users there is no hint of an extended syntax. Anyway, we can consider it as an advanced feature which requires users to study deeper. No problem. Juha -- ___ lazarus mailing list lazarus@lists.lazarus-ide.org https://lists.lazarus-ide.org/listinfo/lazarus
Re: [Lazarus] Masks: ConstructLegacy
On Wed, Oct 27, 2021 at 9:55 PM Juha Manninen via lazarus wrote: > The idea was only to offer an intuitive API which gives a hint there is > something extended available, just like CreateLegacy() gave a hint there is > the good old legacy syntax available. Attached the codetools popup for TMask.Create constructor. I would think it would be clear enough? -- Bart -- ___ lazarus mailing list lazarus@lists.lazarus-ide.org https://lists.lazarus-ide.org/listinfo/lazarus
Re: [Lazarus] Masks: ConstructLegacy
On Wed, Oct 27, 2021 at 10:06 PM Bart via lazarus < lazarus@lists.lazarus-ide.org> wrote: > You totally lost me here. > IMHO there is no need for CreateExtende or similar new constructor. > Why not? THis is what we currently have. > > TMask: > constructor Create(const aMask: String; aCaseSensitive: Boolean; > aOpcodesAllowed: TMaskOpCodes); virtual; overload; > I understood you will change constructor Create() to support the backwards compatible syntax. Now there is CreateLegacy() to make it easy and intuitive to use. The same way a new CreateExtended() would make the new syntax easier to use. Such a constructor is handy and intuitive especially when code completion is used. A user sees alternative constructor names right away. A sign of an intuitive well designed API is that you can select methods and properties etc. with code completion by their names without referring much to documentation. At least you then get an idea of what to search from the documentation. These __are__ the extended constructors for aal the fancy new/extended > stuff. > They are called by all the other constructors that only have the old > parameters. > > So we have backwardscompatibility and extended capability just with > all constructors named simply Create. > The idea was only to offer an intuitive API which gives a hint there is something extended available, just like CreateLegacy() gave a hint there is the good old legacy syntax available. Juha -- ___ lazarus mailing list lazarus@lists.lazarus-ide.org https://lists.lazarus-ide.org/listinfo/lazarus
Re: [Lazarus] Masks: ConstructLegacy
On Wed, Oct 27, 2021 at 8:46 PM Juha Manninen via lazarus wrote: > There would be a constructor named CreateExtended or CreateAdvanced or > similar, allowing the new nice syntax. You totally lost me here. IMHO there is no need for CreateExtende or similar new constructor. THis is what we currently have. TMask: constructor Create(const aMask: String; aCaseSensitive: Boolean; aOpcodesAllowed: TMaskOpCodes); virtual; overload; TMaskWindows: constructor Create(const aMask: String; aCaseSensitive: Boolean; aOpcodesAllowed: TMaskOpCodes; aWindowsQuirksAllowed: TWindowsQuirks); TMaskList: constructor Create(const aValue: String; aSeparator: Char=';'; CaseSensitive: Boolean=False; aOpcodesAllowed: TMaskOpCodes=MaskOpCodesDefaultAllowed); TMaskListWindows: constructor Create(const aValue: String; aSeparator: Char=';'; CaseSensitive: Boolean=False; aOpcodesAllowed: TMaskOpCodes=MaskOpCodesDefaultAllowed; aWindowsQuirksAllowed: TWindowsQuirks=WindowsQuirksDefaultAllowed); reintroduce; These __are__ the extended constructors for aal the fancy new/extended stuff. They are called by all the other constructors that only have the old parameters. So we have backwardscompatibility and extended capability just with all constructors named simply Create. -- Bart -- ___ lazarus mailing list lazarus@lists.lazarus-ide.org https://lists.lazarus-ide.org/listinfo/lazarus
Re: [Lazarus] TMask revisited
On Wed, Oct 27, 2021 at 6:42 PM José Mejuto via lazarus wrote: > Line 780, current: > >Add(TMaskParsedCode.OptionalChar); >Add(fCPLength,@fMask[fMaskInd]); >fLastOC:=TMaskParsedCode.OptionalChar; > > Line 780, new: > >if (mocSet in fMaskOpcodesAllowed) then begin > > Add(TMaskParsedCode.OptionalChar); > Add(fCPLength,@fMask[fMaskInd]); > fLastOC:=TMaskParsedCode.OptionalChar; > >end else begin > Exception_InvalidCharMask(fMask[fMaskInd],fMaskInd); >end; Applied in #63847a62. FWIW: having a proper patchfile makes this a lot easier. Do you have git installed or TortoiseGit? I would much appreciate your patch for the "escaping bug", so I can apply it to main. -- Bart -- ___ lazarus mailing list lazarus@lists.lazarus-ide.org https://lists.lazarus-ide.org/listinfo/lazarus
Re: [Lazarus] Masks: ConstructLegacy
On Wed, Oct 27, 2021 at 6:44 PM Bart via lazarus < lazarus@lists.lazarus-ide.org> wrote: > > The extended syntax would have another constructor. > > Not really sure what you mean by that. > There would be a constructor named CreateExtended or CreateAdvanced or similar, allowing the new nice syntax. Juha -- ___ lazarus mailing list lazarus@lists.lazarus-ide.org https://lists.lazarus-ide.org/listinfo/lazarus
Re: [Lazarus] TMask revisited
El 27/10/2021 a las 13:35, Bart via lazarus escribió: On Wed, Oct 27, 2021 at 1:28 PM José Mejuto via lazarus wrote: "]" must be escaped in all cases, with ranges and with sets or it will be interpreted as a premature closing (ranges). Actually I did not think of that. Could you possibly provide a patch against main and post it on GitLab (or attch the diff here)? Hello, Line 780, current: Add(TMaskParsedCode.OptionalChar); Add(fCPLength,@fMask[fMaskInd]); fLastOC:=TMaskParsedCode.OptionalChar; Line 780, new: if (mocSet in fMaskOpcodesAllowed) then begin Add(TMaskParsedCode.OptionalChar); Add(fCPLength,@fMask[fMaskInd]); fLastOC:=TMaskParsedCode.OptionalChar; end else begin Exception_InvalidCharMask(fMask[fMaskInd],fMaskInd); end; -- -- ___ lazarus mailing list lazarus@lists.lazarus-ide.org https://lists.lazarus-ide.org/listinfo/lazarus
Re: [Lazarus] TMask revisited
El 27/10/2021 a las 13:38, Bart via lazarus escribió: On Wed, Oct 27, 2021 at 1:28 PM José Mejuto via lazarus wrote: This is a side effect of the found bug, in ranges the only valid syntax (without sets enabled) is "char-char". So, without [mocSet] [a-dqx] would be invalid? Hello, With mocRanges active yes, it will be invalid: "T[a-dqx]" => "Td" : Invalid char mask "q" at 6 -- -- ___ lazarus mailing list lazarus@lists.lazarus-ide.org https://lists.lazarus-ide.org/listinfo/lazarus
[Lazarus] Masks: the naming of ...
Hi, I thought I better start a new therad for this one, otherwise I get lost in the previous "TMask revisited" thread. I would like to rename some stuff, now we still can. Easier to remeber IMO: WindowsQuirksAllAllowed -> AllWindowsQuirks WindowsQuirksDefaultAllowed -> DefaultWindowsQuirks MaskOpCodesAllAllowed -> AllMaskOpcodes MaskOpCodesDefaultAllowed -> DefaultMaskOpcodes property RangeAutoReverse -> AutoReverseRange Internal consistency: TMaskWindows -> TWindowsMask TMaskListWindows -> TWindowsMaskList (Because of the Matches* functions we already have) Consitent coding style: TMaskParsedCode enums: mpcXXX TMaskFailCause enums: mfcXXX Opinions please. -- Bart -- ___ lazarus mailing list lazarus@lists.lazarus-ide.org https://lists.lazarus-ide.org/listinfo/lazarus
Re: [Lazarus] Masks: ConstructLegacy
On Wed, Oct 27, 2021 at 2:09 PM Juha Manninen via lazarus wrote: >> Wouldn't is be a bit more logical to exclude mocEscapeChar form the >> MaskOpCodesDefaultAllowed constant, since we'ld like to have the >> default behaviour as compatible as possible? > > > That is fine with me. The Create constructors would then behave like > CreateLegacy now. Yes, that sort of was the objective. > The extended syntax would have another constructor. Not really sure what you mean by that. The "default" constructor is the one with the TMaskOpcodes (/TWindowsQuirks) parameters. To be even more backwards compatible, mocRange should be disabled as well. I'm a bit reluctant to do so, as having ranges is quite an improvement over the old implementation (which had only sets). Possible problem is the '-' inside a range: this would be no problem in old code, but can raise exceptions in the new code. -- Bart -- ___ lazarus mailing list lazarus@lists.lazarus-ide.org https://lists.lazarus-ide.org/listinfo/lazarus
Re: [Lazarus] Masks: ConstructLegacy
On Wed, Oct 27, 2021 at 2:50 PM Bart via lazarus < lazarus@lists.lazarus-ide.org> wrote: > The new masks unit has several CreateLegacy constructors (and some > *Legacy* functions). > They call the new constructros with mocEscapeChar disabled. > > Wouldn't is be a bit more logical to exclude mocEscapeChar form the > MaskOpCodesDefaultAllowed constant, since we'ld like to have the > default behaviour as compatible as possible? > That is fine with me. The Create constructors would then behave like CreateLegacy now. The extended syntax would have another constructor. I was pondering this same topic in an earlier post. Juha -- ___ lazarus mailing list lazarus@lists.lazarus-ide.org https://lists.lazarus-ide.org/listinfo/lazarus
[Lazarus] Masks: ConstructLegacy
Hi, The new masks unit has several CreateLegacy constructors (and some *Legacy* functions). They call the new constructros with mocEscapeChar disabled. Wouldn't is be a bit more logical to exclude mocEscapeChar form the MaskOpCodesDefaultAllowed constant, since we'ld like to have the default behaviour as compatible as possible? Since most of this code will be used for filename matching, escaping in most common practice won't be necessary. Also, in existing code people may have mask with '\' as intentionally as a PathDelim, and this would break that. If we leave things as is, then the question remains: how long do we keep the *Legacy* constructors and functions? -- Bart -- ___ lazarus mailing list lazarus@lists.lazarus-ide.org https://lists.lazarus-ide.org/listinfo/lazarus
Re: [Lazarus] TMask revisited
On Wed, Oct 27, 2021 at 1:28 PM José Mejuto via lazarus wrote: > This is a side effect of the found bug, in ranges the only valid syntax > (without sets enabled) is "char-char". So, without [mocSet] [a-dqx] would be invalid? -- Bart -- ___ lazarus mailing list lazarus@lists.lazarus-ide.org https://lists.lazarus-ide.org/listinfo/lazarus
Re: [Lazarus] TMask revisited
On Wed, Oct 27, 2021 at 1:28 PM José Mejuto via lazarus wrote: > "]" must be escaped in all cases, with ranges and with sets or it will > be interpreted as a premature closing (ranges). Actually I did not think of that. Could you possibly provide a patch against main and post it on GitLab (or attch the diff here)? -- Bart -- ___ lazarus mailing list lazarus@lists.lazarus-ide.org https://lists.lazarus-ide.org/listinfo/lazarus
Re: [Lazarus] TMask revisited
El 26/10/2021 a las 19:01, Bart via lazarus escribió: On Tue, Oct 26, 2021 at 6:48 PM Bart wrote: Point 2 would need (probably a minor) change to the CompileRange method. Attached diff might do what I intended. @José: does it in fact allow ? in a range as a literal, without side effects. I don't really understand the matching algorithm. Hello, Only ranges and escape char activated. Mask String "T[?]" => "T?" : Invalid char mask "?" at 3 "T[0-?]" => "T?" : True "T[?]" fails because in ranges valid syntax is only "char-char". -- -- ___ lazarus mailing list lazarus@lists.lazarus-ide.org https://lists.lazarus-ide.org/listinfo/lazarus
Re: [Lazarus] New TMaskList [forked from: TMask revisited]
On Tue, Oct 26, 2021 at 10:44 PM Bart wrote: > I'll have a go at it then To simplify matters I decided to remove the CreateWindows and CreateNative constructors for TMaskList. The CreateWindows skipped the population of fMasksWindows, but that is a small price to pay IMO. I can't have this as a constructor in TMaskListWindows, but I would inherit it, so I got rid of it. So, for the moment TMaskList always internally creates and populates 2 lists. I tried to use a factory pattern, , so that I would not have to replicate the Matches method for TMaskListWindows. I factored out populating the respective objeclists (fMasks and fWindowsMasks) and made that a virtual method so I could override that in TMaskListWindows. That has it's drawbacks however: in TMaskListWindows I have to iterate through fMasks in order to set some privae variables/fields of the created TMaskWindows instances, and then have to call the Compile method. (Duplicating some actions in the TMaskWindows constructor, so that possibly needs updating when TMaskWindows constructor gets changed). The problem turned out to be the fact that: FMaskClass.Create(S[i], CaseSensitive, aOpcodesAllowed) does not call the TMaskUtf8Windows constructor, when FMaskClass definitely _is_ TMaskWindows (it calls TMaskUTF8 constructor directly). This probably is because the signature of te constructor has a long list of default parameters. Unfortunately you can't override that constructor in TMaskUtf8Windows: you'll get a "Error: Can't determine which overloaded function to call". So, I untangeld all constructors for TMaskUtf8: constructor Create(const aMask: String); (1) constructor Create(const aMask: String; aCaseSensitive: Boolean); (2) constructor Create(const aMask: String; aCaseSensitive: Boolean; aOpcodesAllowed: TMaskOpCodes); virtual; overload; (3) Then in TMaskUtf8Windows I override the last one (3) to call the constructor with "quirks" parameter. It may look a bit messy, but AFAICS all possible constructor calls still work. (I had to add "overload" to constructor (3), otherwise constructors (1) and (2) were not visible in TMaskWindowsUTF8, something about constructors which I did not know). -- Bart -- ___ lazarus mailing list lazarus@lists.lazarus-ide.org https://lists.lazarus-ide.org/listinfo/lazarus
Re: [Lazarus] TMask revisited
El 26/10/2021 a las 18:48, Bart via lazarus escribió:
On Tue, Oct 26, 2021 at 1:38 PM José Mejuto via lazarus
wrote:
You found a bug,
3. '-' if it is NOT the first char (or the first after the negating
!), it is then the indicator for a range
Hello,
This is a side effect of the found bug, in ranges the only valid syntax
(without sets enabled) is "char-char".
So, even '*' is taken as a literal.
Escaping must work the same way (if possible) in all the mask, changing
behaviour is not desirable as creates confusion. Sometimes when I write
a mask in RegEx I escape some chars even when they are not needed to be
escaped because it adds some clarity to the expression, cos if it is
escaped I know for sure its a literal and no need to check if it matches
a previous "[" or "{" or "(".
A '?' in a range is forbidden (EMaskError: Invalid character "?" in
mask), except for the [?].
It must be escaped to have meaning.
Given this, I would think that:
1. escaping inside a range is not necessary: everything in a range is
literal except for the 3 cases outlined above.
2. having a '?' could also be accepted as long as it is not the first
character in the range specification, even [!?] could be allowed: any
char but not a '?'.
Point 2 would need (probably a minor) change to the CompileRange method.
With the last changed commited in GitHub (fixing the bug) escape would
be necessary:
Macth strings "[Hell]" and "[Hell["
Mask: "*[\]\[]"
"]" must be escaped in all cases, with ranges and with sets or it will
be interpreted as a premature closing (ranges).
--
--
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus
