Re: [Lazarus] TMask revisited

[José Mejuto via lazarus]

El 27/10/2021 a las 20:57, Bart via lazarus escribió:


  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?


Hello,

I know, I'm using it, but setup a repo just to insert 3 lines to 
generate a .patch is a bit of overengineer :-)



I would much appreciate your patch for the "escaping bug", so I can
apply it to main.


[thumbs up] :-)

--

--
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

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] TMask revisited

[José Mejuto via lazarus]

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

[José Mejuto via lazarus]

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

Re: [Lazarus] TMask revisited

[Bart via lazarus]

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

[Bart via lazarus]

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

[José Mejuto via lazarus]

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] TMask revisited

[José Mejuto via lazarus]

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

Re: [Lazarus] TMask revisited

[Bart via lazarus]

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.

-- 
Bart


masks.allowquestionmarkinrange.diff
Description: Binary data
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Tue, Oct 26, 2021 at 1:38 PM José Mejuto via lazarus
 wrote:

> You found a bug,

I thought about it some more.
Inside a range everything is treated as a literal, the only exceptions are:
1. [?]
2. '!' as the first char in a range when [mocNegateGroup] is enabled.
3. '-' if it is NOT the first char (or the first after the negating
!), it is then the indicator for a range

So, even '*' is taken as a literal.
Since '*' as a wildchar in a range would make no sense at all, to me
this sounds OK.
A '?' in a range is forbidden (EMaskError: Invalid character "?" in
mask), except for the [?].
Having '?' interpreted as a wildcard in a range would also make no sense.
The only things one could possibly imagine to have to escape in a range are:
1. ? for a literal ?: currently forbidden
2. * for a literal *: currently * works as a literal
3. - for a literal -: solvable by having -  as the first char in a range.
4. The EscapeChar itself.

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.

It would then be simply a matter of documenting this.

-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[José Mejuto via lazarus]

El 24/10/2021 a las 15:48, Bart via lazarus escribió:


It looks like escaping does not work as advertised?

Seems like escaping is NOT supported in ranges or sets, but only outside them?
If that is the case (and by design) then, with [mocRange] enabled, you
can only have '-' in a range if the range starts with '-'?


Hello,

You found a bug, there is a missing check in the Compile procedure, just 
in the last lines of the function, where if every previous check fails 
it assumes a "CharInASet" (OptionalChar) and it must check if 
OptionalChars are allowed and if they are not add it as literal. The fix 
is quite easy:


-
  end else begin
if (lMask[j]=FMaskEscapeChar) and 
(eMaskOpcodeEscapeChar in FMaskOpcodesAllowed) then begin

  // next is optional char in set or literal
  inc(j,lCPLength);
  if j<=cMaskLimit then begin
lCPLength:=UTF8Length(@lMask[j]);
  end else begin
Exception_IncompleteMask();
  end;
end;
//FIX
if eMaskOpcodeOptionalChar in FMaskOpcodesAllowed 
then begin

  Add(TMaskOpCode.OptionalChar);
  Add(lCPLength,@lMask[j]);
  lLast:=TMaskOpCode.OptionalChar;
end else begin
  Add(TMaskOpCode.Literal);
  Add(lCPLength,@lMask[j]);
  inc(cMatchMinimumLiteralBytes,lCPLength);
  if 
cMatchMaximumLiteralBytesinc(cMatchMaximumLiteralBytes,lCPLength);

  lLast:=TMaskOpCode.Literal;
end;
//FIX-END
  end;
  inc(j,lCPLength);
--

Anyway it should be more clear in the GitHub repo when push the update.

--

--
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Juha Manninen via lazarus]

On Mon, Oct 25, 2021 at 7:20 PM Bart via lazarus <
lazarus@lists.lazarus-ide.org> wrote:

> As I see it now, we are planning to remove all the old TMask stuff (in
> the future) and replace it with the new and improved TMask.
>
> Shouldn't that mean that we should also deprecate the CreateLegacy method?
> That way we can remove all backwards compatibility stuff in 2.5/2.6.
> And the same for the TMaskOption(s) type? (Although it stands to
> reason that we remove this once we remove the methods that use those
> as a paramater-type.)
>

I personally would like to keep the CreateLegacy constructor, or a similar
one with another name.
Users who are accustomed to the old syntax may find it useful.
I leave it for you to decide.
I was thinking hard if I should name the current CreateLegacy as Create and
then have another constructor for the improved / extended syntax. That
would be more backwards compatible.
I am happy with both solutions.
Fortunately the backwards compatibility issue is not serious because the
vast majority of masks are simple *.txt or similar and they keep on working.

Juha
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Wed, Oct 20, 2021 at 11:42 AM Bart  wrote:

> > The Create in TMaskBase is never called directly by a user. He will get a 
> > deprecated message from elsewhere.

As I see it now, we are planning to remove all the old TMask stuff (in
the future) and replace it with the new and improved TMask.

Shouldn't that mean that we should also deprecate the CreateLegacy method?
That way we can remove all backwards compatibility stuff in 2.5/2.6.
And the same for the TMaskOption(s) type? (Although it stands to
reason that we remove this once we remove the methods that use those
as a paramater-type.)

-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Sun, Oct 24, 2021 at 4:26 PM José Mejuto via lazarus
 wrote:

> > @José: is this indeed as intended?

> No, in fact escaping was introduced to allow "[a\-]" to be interpreted
> as literal set "a-". I must check my test cases, maybe a simple missing
> if. Please let me check it tomorrow, monday.

OK, thanks


About mocRange and mocSet (previously AdditionalChar).
Since ranges allow/include sets (e.i. [abc] is a valid range), and the
[mocSet] is only used in Compile, but there mocRange follows the same
code-path:
IMO mocSet can be removed.
Unless, of course [abc] is NOT supposed to be a valid range...

-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[José Mejuto via lazarus]

El 24/10/2021 a las 15:48, Bart via lazarus escribió:


It looks like escaping does not work as advertised?

Seems like escaping is NOT supported in ranges or sets, but only outside them?
If that is the case (and by design) then, with [mocRange] enabled, you
can only have '-' in a range if the range starts with '-'?
@José: is this indeed as intended?
If so, I'll adjust the comments for the mocXXX enums.



Hello,

No, in fact escaping was introduced to allow "[a\-]" to be interpreted 
as literal set "a-". I must check my test cases, maybe a simple missing 
if. Please let me check it tomorrow, monday.


--

--
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Sun, Oct 24, 2021 at 3:26 PM Bart  wrote:

> It looks like escaping does not work as advertised?

Seems like escaping is NOT supported in ranges or sets, but only outside them?
If that is the case (and by design) then, with [mocRange] enabled, you
can only have '-' in a range if the range starts with '-'?

In essence that's fine with me.

@José: is this indeed as intended?
If so, I'll adjust the comments for the mocXXX enums.

-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Sat, Oct 23, 2021 at 11:33 PM Bart  wrote:


> I renamed mocOptionaChar to mocSet and added some comments in the code.
>
> @José: are these comments correct?


I'm still strugling with the difference between mocRange and mocSet
(previously mocOptionalChar).
Consider the following mask: [a-c]
With [mocRange] this matches 'a', 'b', or 'c', but not '-'.
With [mocSet] this matches 'a', '-' or 'c', but not 'b'.
That's clear to me.

Mask [ac]
[mocRange]: matches 'a' or 'c'.
[mocSet]: matches 'a' or 'c'.
To me this seems as [mocRange] is equivalent to  [mocRange,mocSet]
So, only mocSet without mocRange has a special meaning.
Or they should be mutually exclusive: if you enable mocSet, mocRange
should be disabled and vice versa (the latter does not cause any
functional change).

Now we take into account the mocEscapeChar and things get a little confusing.
IIUC then if mocEscapeChar is eneabled, then EscapeChar+SomeChar is
interpreted as just a literal SomeChar.
By default EscapeChar = '\' so '\*' is  a literal '*' and '\\' is a literal '\'.
And '\-' should mean a literal '-'.

In the following examples RangeAutoReverse is set to FALSE.

Opcodes : [mocRange]
Mask: [a\-]
Filename: -
EMaskError: Missing closing character "]" in mask (offset 5).
This is to be expected.

Opcodes : [mocRange,mocEscapeChar]
Mask: [a\-]
Filename: -
EMaskError: Missing closing character "]" in mask (offset 5).  ** ?
Escaping the '-' should mean that the mask should match 'a' or '-'

Opcodes : [mocSet]
Mask: [a\-]
Filename: \
Matches
This is expected

Opcodes : [mocSet,mocEscapeChar]
Mask: [a\-]
Filename: \
Matches
Since mocEscapeChar is enabled the sequence '\-' should be interpreted
as a literal '-' and it should therefore not match '\'.

Some more oddities:
Opcodes : [mocRange]
Mask: [$\-&]
Filename: $
Matches

Opcodes : [mocRange]
Mask: [$\-&]
Filename: %
Does NOT match

Opcodes : [mocRange]
Mask: [$\-&]
Filename: &
Does NOT match
Actually OK, since the range '\-&' is not reversed and therefore empty
(took me a while to realize that)


Opcodes : [mocRange]
Mask: [$\-&]
Filename: -
Does NOT match

Opcodes : [mocRange]
Mask: [$\-&]
Filename: \
Does NOT match



Opcodes : [mocRange,mocEscapeChar]
Mask: [$\-&]
Filename: $
Matches

Opcodes : [mocRange,mocEscapeChar]
Mask: [$\-&]
Filename: %
Does NOT match

Opcodes : [mocRange,mocEscapeChar]
Mask: [$\-&]
Filename: &
Does NOT match   ?
Looks like '\-&' is interpreted as an (empty) range, but '\-' should
be interpreted as a literal '-', so not a range but a set, and it
should match

Opcodes : [mocRange,mocEscapeChar]
Mask: [$\-&]
Filename: -
Does NOT match   ?
Same argument as above

Opcodes : [mocRange,mocEscapeChar]
Mask: [$\-&]
Filename: \
Does NOT match


With [mocSet] instead of [mocRange]

Opcodes : [mocSet]
Mask: [$\-&]
Filename: \
Matches


Opcodes : [mocSet,mocEscapeChar]
Mask: [$\-&]
Filename: \
Matches  ?
Here enabling mocEscapeChar makes no difference at all. (Checked the
same filenames as with mocRange)

Finally this:
Opcodes : [mocRange]
Mask: [\a-c]
Filename: \
Matches

Opcodes : [mocRange,mocEscapeChar]
Mask: [\a-c]
Filename: \
Matches  ?
It should not.

It looks like escaping does not work as advertised?
-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Sat, Oct 23, 2021 at 10:39 PM Juha Manninen via lazarus
 wrote:

> In 964d5f4d69 I changed most names as you suggested.
> The original TMaskOpCode is now TMaskParsedCode because it is not related to 
> the other enums directly.
> I did the changes before reading your last mail.

I renamed mocOptionaChar to mocSet and added some comments in the code.

@José: are these comments correct?
  TMaskOpCode=(mocAnyChar,  //treat ? as a wildcard
   mocAnyCharOrNone,//treat [?] to match any char the
absence of a char
   mocAnyText,  //treat * as a wildcard
   mocRange,//treat [a-d] to match either 'a',
'b', 'c', or 'd'
   mocSet,  //treat [ab] to match either 'a' or 'b
   mocNegateGroup,  //treat [!a-d] to not match 'a',
'b', 'c', or 'd', but match any other char. Requires mocRange and/or
mocSet
   mocEscapeChar);  //treat EscapeChar (defaults to
'\') in [a\-b] to match either 'a', 'b', or '-'. Requires
mocOptionalChar enabled and mocRange disabled

-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Tue, Oct 19, 2021 at 10:44 AM José Mejuto via lazarus
 wrote:

> With "eMaskOpcodeRange" and "eMaskOpcodeOptionalChar" enabled to match
> "a" or "-" or "z" the "-" must be escaped (something like regex) using
> the escapechar, by default "\", in this way "[a\-z]".

That does not seem to work (at least not as I expected):
Opcodes: [eMaskOpcodeRange, eMaskOpcodeOptionalChar, eMaskOpcodeEscapeChar]
Mask: [a\-z]  // since '-' is escaped should match 'a', '-' or 'z'
Matches Filenames 'a', 'b', 'c' ,'d' .. 'z', but not '-'

Opcodes: [eMaskOpcodeRange, eMaskOpcodeOptionalChar, eMaskOpcodeEscapeChar]
Mask: [a\-]   //should match either 'a' or '-'
EMaskError: Missing closing character "]" in mask (offset 5).

Opcodes: [eMaskOpcodeOptionalChar, eMaskOpcodeEscapeChar]
Mask: [a\-]   //should match either 'a' or '-'
Matches Filenames: 'a', '\' and '-'
Q: eMaskOpcodeEscapeChar only treats '\' as escape char is
eMaskOpcodeRange is enabled?

-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[José Mejuto via lazarus]

El 23/10/2021 a las 22:18, Bart via lazarus escribió:


Q2: @José: the TMaskOpCode enums are indexed, and there is a gap in
the index. Is the index necessary for the code to work?
Is the gap necessary?
In the Add() method, the enum is cast to a byte, this should also work
if no indices are used (as long as you do not do mathematical
operations with the value of that byte).


Hello,

The value was given just to group some things, originally they were 
constants, in theory values can be removed or changed (unless I forget 
something), they are used as a marker in compiled stream, like a command.



--

--
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Juha Manninen via lazarus]

On Sat, Oct 23, 2021 at 11:21 PM Bart via lazarus <
lazarus@lists.lazarus-ide.org> wrote:

> AFAICS the TMaskOpCode and TMaskOpcodesEnum types are not related at all.
> TMaskOpCode is some kind of internal identifier in the internal mask
> representation, whereas TMaskOpcodesEnum are options for interpreting
> the mask.
>
> Q1: is it possible to have the TMaskOpCode as an internal type in the
> class (maybe in the protected part of the class definition)?
> It seems it is not needed outside (as in the public part of the interface).
>
> If we did not already have the old TMaskOption type, I would actually
> have liked to propose to rename the TMaskOpcodesEnum into
> TMaskOption...
>

In 964d5f4d69 I changed most names as you suggested.
The original TMaskOpCode is now TMaskParsedCode because it is not related
to the other enums directly.
I did the changes before reading your last mail.
I remember many enums actually are class internals in José's original code
(in GitHub). I moved them outside to create the constants more easily.
Maybe they can be moved back to a class. You can experiment with it.
You can rename other stuff, too, if you like. I will not touch it for a
while now.

Juha
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Sat, Oct 23, 2021 at 12:22 PM Bart  wrote:

> Then we have TMaskOpcode and TMaskOpcodesEnum types.
> The first one is more or less an internal type.
> The latter one is for common user interface.
> Since TMaskOpCode is used in the interface part of TMask, we must have
> it in the interface part of the unit.

I've given it some more thought.

AFAICS the TMaskOpCode and TMaskOpcodesEnum types are not related at all.
TMaskOpCode is some kind of internal identifier in the internal mask
representation, whereas TMaskOpcodesEnum are options for interpreting
the mask.

Q1: is it possible to have the TMaskOpCode as an internal type in the
class (maybe in the protected part of the class definition)?
It seems it is not needed outside (as in the public part of the interface).

If we did not already have the old TMaskOption type, I would actually
have liked to propose to rename the TMaskOpcodesEnum into
TMaskOption...


Q2: @José: the TMaskOpCode enums are indexed, and there is a gap in
the index. Is the index necessary for the code to work?
Is the gap necessary?
In the Add() method, the enum is cast to a byte, this should also work
if no indices are used (as long as you do not do mathematical
operations with the value of that byte).

-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Sat, Oct 23, 2021 at 6:16 PM José Mejuto via lazarus
 wrote:

> Because in the code each syntax piece can be enabled and disabled, even
> "*" and "?" can be disabled to not be interpreted as a mask char, so to
> allow granularity a name to that "feature" must be given.

Yep, OK.

-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[José Mejuto via lazarus]

El 23/10/2021 a las 12:02, Bart via lazarus escribió:


I can understand why having [a-e] is good (and better than [abcde]).
I was just confused by the fact that these had different names.


Hello,

Because in the code each syntax piece can be enabled and disabled, even 
"*" and "?" can be disabled to not be interpreted as a mask char, so to 
allow granularity a name to that "feature" must be given.



--

--
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[José Mejuto via lazarus]

El 23/10/2021 a las 12:22, Bart via lazarus escribió:

On Sat, Oct 23, 2021 at 12:02 PM Bart  wrote:


since we are still looking for a better (?) name for the
eMaskOpcodeOptionalChar enum:


This brings me to another point, and please, please, please don't see
this as criticism of feel offended by me.


Hello,

Trying to help, my variables notation (used across several languages) is:

c* = Class variable
l* = Local variable
F* = Class published variable
T* = Type, struct, record, set...
e* = Enum value
p* = Pointer

Each language have its style rules, but as I'm using different languages 
(as many people) designed some little rules to help me in the future not 
to the style guide designers. Fortunately Lazarus IDE makes rename them 
very, very easy.


--

--
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Sat, Oct 23, 2021 at 12:22 PM Bart  wrote:

> So:
> TInternalMaskOpcode (integers)
> TMaskOpcode (the enums)
> TMaskOpcodes: set of TMaskOpcode
> Enum names: moXXX

Maybe better make that mopXXX, as not to confuse them with old moXXX
TMaskOption enums.

-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Maxim Ganetsky via lazarus]

23.10.2021 13:22, Bart via lazarus пишет:

On Sat, Oct 23, 2021 at 12:02 PM Bart  wrote:


since we are still looking for a better (?) name for the
eMaskOpcodeOptionalChar enum:


This brings me to another point, and please, please, please don't see
this as criticism of feel offended by me.

Naming conventions.
Typically we don't have the habit of haveing the literal phrase "enum"
in our enum typenames.
We also tend to adopt that enum names are derived form the typename in
a manner like
typename: TSomeType
enum name: stXXX

Then we have TMaskOpcode and TMaskOpcodesEnum types.
The first one is more or less an internal type.
The latter one is for common user interface.
Since TMaskOpCode is used in the interface part of TMask, we must have
it in the interface part of the unit.

If we rename TMaskOpCode to TInternalMaskOpCode and rename
TMaskOpcodesEnum to TMaskOpcodesEnum to TMaskOpcode and
TMaskOpcodesSet to TMaskOpcodes (we tend to do that with sets of
enums: set typename = typename+s), then the opcode enum names can be
changed to moAnyChar, moAnyCharOrNone etc.
So:
TInternalMaskOpcode (integers)
TMaskOpcode (the enums)
TMaskOpcodes: set of TMaskOpcode
Enum names: moXXX

This would be more in line with our unwritten style guide (at least
this is how I percieve it).
Also we have shorter, yet not less intuitive, enum names.

The same for TWindowsQuirks:
TWindowsQuirks: enums
TWindowsQuirks: set of TWindowsQuirks
Enum names: wqXXX

Yes, those are intrusive changes, and they add no functionality, nor
do they fix any bugs, but this is the time to consider such a change.
The new and improved (yes, improved) TMask with all new types won't be
used very much ATM.
In a few weeks or months this might not be the case anymore.

Opinions please!


Makes sense.

--
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] TMask revisited

[Bart via lazarus]

On Sat, Oct 23, 2021 at 12:22 PM Bart  wrote:

> Naming conventions.

Also: we typically have the convention of nameing fileds in a class Fxxx
Here we have cXXX and eXXX.

Again: not meant as crtitcism.

-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Sat, Oct 23, 2021 at 12:02 PM Bart  wrote:

> since we are still looking for a better (?) name for the
> eMaskOpcodeOptionalChar enum:

This brings me to another point, and please, please, please don't see
this as criticism of feel offended by me.

Naming conventions.
Typically we don't have the habit of haveing the literal phrase "enum"
in our enum typenames.
We also tend to adopt that enum names are derived form the typename in
a manner like
typename: TSomeType
enum name: stXXX

Then we have TMaskOpcode and TMaskOpcodesEnum types.
The first one is more or less an internal type.
The latter one is for common user interface.
Since TMaskOpCode is used in the interface part of TMask, we must have
it in the interface part of the unit.

If we rename TMaskOpCode to TInternalMaskOpCode and rename
TMaskOpcodesEnum to TMaskOpcodesEnum to TMaskOpcode and
TMaskOpcodesSet to TMaskOpcodes (we tend to do that with sets of
enums: set typename = typename+s), then the opcode enum names can be
changed to moAnyChar, moAnyCharOrNone etc.
So:
TInternalMaskOpcode (integers)
TMaskOpcode (the enums)
TMaskOpcodes: set of TMaskOpcode
Enum names: moXXX

This would be more in line with our unwritten style guide (at least
this is how I percieve it).
Also we have shorter, yet not less intuitive, enum names.

The same for TWindowsQuirks:
TWindowsQuirks: enums
TWindowsQuirks: set of TWindowsQuirks
Enum names: wqXXX

Yes, those are intrusive changes, and they add no functionality, nor
do they fix any bugs, but this is the time to consider such a change.
The new and improved (yes, improved) TMask with all new types won't be
used very much ATM.
In a few weeks or months this might not be the case anymore.

Opinions please!

-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Thu, Oct 21, 2021 at 10:29 AM José Mejuto via lazarus
 wrote:

> So the question is, why sets if ranges can be used ? Because sometimes
> you need to exclude strings that starts with number 1:
>
> "[0234567989][a-z]"
>
> > Naming them different just confuses me (which probably is my fault).
>
> Because they are different beast in the code and in the syntax, many
> times ranges apparently are syntactic sugar for sets, but ranges have a
> begin and an end, and both have a mission, one, ranges, add clarity to
> the desired match, and the second, sets, add flexibility to the mask write.
>

I can understand why having [a-e] is good (and better than [abcde]).
I was just confused by the fact that these had different names.

since we are still looking for a better (?) name for the
eMaskOpcodeOptionalChar enum:
When eMaskOpcodeOptionalChar enables the use of [abcde] and
eMaskOpcodeRange enables the use of [a-z] and syntactically there is a
difference between ranges [a-e] and sets [abcde], then a potentially
better name could be eMaskOpcodeSet(s)?

-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Juha Manninen via lazarus]

On Thu, Oct 21, 2021 at 11:30 AM José Mejuto via lazarus <
lazarus@lists.lazarus-ide.org> wrote:

> Now you need to match a string that starts with a number and the second
> char must be a letter, with a set it will be too large and unclear:
>
> "[0123456789][abcdefghijklmnopqrstuvwxyz]*"
>

With all Unicode characters included it would be even larger.  :)
Your code supports full Unicode also in the mask itself which is impressive.

I would like to get opinions of the "Legacy" constructors and functions in
the current code.
They use constant MaskOpCodesNoEscape which makes the mask behave more
close to the earlier mask implementation.
For example :
 constructor TMaskUTF8.CreateLegacy(const aMask: String; aCaseSensitive:
Boolean);
and
 function MatchesMaskLegacy(const FileName, Mask: String; CaseSensitive:
Boolean): Boolean;

As the comment for MaskOpCodesNoEscape says:
  // Interpret [?] as literal question mark instead of 0..1 chars wildcard.
  // Disable backslash escaping characters like "\?".
  // Leave out eMaskOpcodeAnyCharOrNone and eMaskOpcodeEscapeChar

The constructors and functions without "Legacy" allow backslash escaping
and treat [?] as a wildcard.
It can be seen as the default behavior because "Create" is the default
constructor name by convention. It is also the default behavior in José's
original code.
I allowed it because the new syntax is an improvement. It is intuitive and
more expressive.
Now, should we emphasize backwards compatibility and use
MaskOpCodesNoEscape by default?
The new improved syntax would then have constructors and functions with
different names, "Extended" maybe.
How does Delphi do it? Should we be Delphi compatible here?
Interestingly there are no standards for Mask syntax. They have evolved and
diverted freely. Thus we cannot follow standards.

Juha
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[José Mejuto via lazarus]

El 20/10/2021 a las 22:59, Bart via lazarus escribió:


Range syntax: [a-z]
Set syntax: [abcdefghijklmnopqrstuvwxyz]


What would be the effective difference between [a-e] and [abcde] then?
I did understand that both meant: a single charcter being either 'a',
'b', 'c', 'd', or 'e'?


Hello,

Technically the set requires one check for every possible char and a 
range only two checks, but the most important one is the easy to write 
for every day situations. It is quite usual to try to match strings that 
starts with a number and not with a letter, so you write a mask like:


"[0-9]*"

Of course you can write it with sets:

"[0123456789]*"

Now you need to match a string that starts with a number and the second 
char must be a letter, with a set it will be too large and unclear:


"[0123456789][abcdefghijklmnopqrstuvwxyz]*"

with ranges:

"[0-9][a-z]*"

So the question is, why sets if ranges can be used ? Because sometimes 
you need to exclude strings that starts with number 1:


"[0234567989][a-z]"


Naming them different just confuses me (which probably is my fault).


Because they are different beast in the code and in the syntax, many 
times ranges apparently are syntactic sugar for sets, but ranges have a 
begin and an end, and both have a mission, one, ranges, add clarity to 
the desired match, and the second, sets, add flexibility to the mask write.


Most times when you need support for one also need for the other, so 
when needs to be disabled, the other, quite sure, needs to be disabled also.


--

--
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Wed, Oct 20, 2021 at 9:37 PM José Mejuto via lazarus
 wrote:

> >> There are IMHO two front lines, one is the "replace" of TMask in
> >> internal LCL functions, exposed or not to the user, and in this case all
> >> options that allow mimic the old behaviour should be disabled. The other
> >> one is the TMask itself which can be "replaced" with same settings as
> >> internal LCL functions and/or a TMaskExtended which can use all the
> >> syntax options.

The IDE just uses the TMask of the LCL (but maybe with restricted
options, so ranges are disabled, as not to break backwards
compatibility.
Then again we might decied to use the full capability of TMask and
give users the option to uses ranges as well.
B.t.w. I did not study that piece of code in the IDE: if it did not
specify moDisableSets, it would have supported ranges (not as extended
as the current implementation though).

Oh, and thank you for donating this code to us.
-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Wed, Oct 20, 2021 at 9:37 PM José Mejuto via lazarus
 wrote:

> In the "masks" world sets are a group of chars inside "[]", optional
> chars, option chars, or other fancy name.
>
> Range syntax: [a-z]
> Set syntax: [abcdefghijklmnopqrstuvwxyz]

What would be the effective difference between [a-e] and [abcde] then?
I did understand that both meant: a single charcter being either 'a',
'b', 'c', 'd', or 'e'?

Naming them different just confuses me (which probably is my fault).

-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[José Mejuto via lazarus]

El 20/10/2021 a las 11:34, Bart via lazarus escribió:


There are IMHO two front lines, one is the "replace" of TMask in
internal LCL functions, exposed or not to the user, and in this case all
options that allow mimic the old behaviour should be disabled. The other
one is the TMask itself which can be "replaced" with same settings as
internal LCL functions and/or a TMaskExtended which can use all the
syntax options.



I'm sorry, but I don't really know what you mean here.


Hello,

Sorry, my English is... BAD :-) I mean the uses of TMask in the IDE, in 
example, where the user of "Find in files" (another example) expects 
simple masks to be used like "?" and "*" only, no ranges, no sets, no 
escape chars, etc. Ans in the other hand a class (maybe TMaskExtended) 
which the programmer can use full features.



but current TMask allows ranges and
sets which is confusing me about the function requirements.

Again ,not sure what you mean by that.
In the old TMask I introduced the option to disable that and called it
moDisableSets (I'm also not native English), where your use of the
word Range might have beeen more appropriate.

In Pascal a set (of characters) is ['a'..'z'], whic is a synatx TMask
does not support AFAICS.


In the "masks" world sets are a group of chars inside "[]", optional 
chars, option chars, or other fancy name.


Range syntax: [a-z]
Set syntax: [abcdefghijklmnopqrstuvwxyz]

I think you are talking about TMask use inside IDE and LCL functions, 
meanwhile I'm talking about TMask capabilities.



--

--
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Tue, Oct 19, 2021 at 7:11 PM Juha Manninen via lazarus
 wrote:

> The Create in TMaskBase is never called directly by a user. He will get a 
> deprecated message from elsewhere.

But it is part of the interface.
Some poor soul might implement it's own dereive control from TMaskBase
and rely on it.

I also suppose that, once the MatchesWindowsMask method has been
removed and the old TMaskOptions (2.6 or later), then that constructor
will be removed as well?
If so, then at least we should comment this in the source code.


-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Wed, Oct 20, 2021 at 11:00 AM José Mejuto via lazarus
 wrote:

> There are IMHO two front lines, one is the "replace" of TMask in
> internal LCL functions, exposed or not to the user, and in this case all
> options that allow mimic the old behaviour should be disabled. The other
> one is the TMask itself which can be "replaced" with same settings as
> internal LCL functions and/or a TMaskExtended which can use all the
> syntax options.

I'm sorry, but I don't really know what you mean here.

> > Lots of documentation to do.
>
> In the mask world I think that many mask samples are better than a lot
> of text trying to explain what a set, a range, etc, is. Short text with
> 3, 4, ... 10 samples.

I agree.

> but current TMask allows ranges and
> sets which is confusing me about the function requirements.

Again ,not sure what you mean by that.
In the old TMask I introduced the option to disable that and called it
moDisableSets (I'm also not native English), where your use of the
word Range might have beeen more appropriate.

In Pascal a set (of characters) is ['a'..'z'], whic is a synatx TMask
does not support AFAICS.

-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[José Mejuto via lazarus]

El 19/10/2021 a las 16:32, Bart via lazarus escribió:


Thanks for explaining that.
Sounds to me like you should not enable both of them, unless you want
something to match 'a'..'f' or '+' or '-' [a-f+\-] ?


Hello,

There are IMHO two front lines, one is the "replace" of TMask in 
internal LCL functions, exposed or not to the user, and in this case all 
options that allow mimic the old behaviour should be disabled. The other 
one is the TMask itself which can be "replaced" with same settings as 
internal LCL functions and/or a TMaskExtended which can use all the 
syntax options.



Lots of documentation to do.


In the mask world I think that many mask samples are better than a lot 
of text trying to explain what a set, a range, etc, is. Short text with 
3, 4, ... 10 samples.



The eMaskOpcodeOptionalChar enum name is not very self-explanatory,
maybe we should come up with another name (better now than in half a
year).


For me, a non English speaker, the name is fine, but maybe 
"eMaskOpcodeCharInSet" ?


I'm using "Char" instead CodePoint on purpose.


And maybe a constant that has all opcodes except the ones that enable ranges.


And the one that escape characters, but current TMask allows ranges and 
sets which is confusing me about the function requirements.


--

--
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Juha Manninen via lazarus]

On Tue, Oct 19, 2021 at 5:34 PM Bart via lazarus <
lazarus@lists.lazarus-ide.org> wrote:

> With only eMaskOpcodeOptionalChar enabled [a-z] would match 'a', '-' or
> 'z'?
> (This would be the old behaviour of TMask)
>

There is now a constant MaskOpCodesNoEscape which leaves out
eMaskOpcodeAnyCharOrNone and eMaskOpcodeEscapeChar.
It matches more or less with the old behavior and is used in the ...Legacy
constructors and functions.

The eMaskOpcodeOptionalChar enum name is not very self-explanatory,
> maybe we should come up with another name (better now than in half a
> year).
>

Suggestions?

And maybe a constant that has all opcodes except the ones that enable
> ranges.
>

There is MaskOpCodesDisableRange which leaves out eMaskOpcodeAnyCharOrNone,
eMaskOpcodeRange and eMaskOpcodeOptionalChar.
It is used in unit ShellCtrls.


I forgot to answer this earlier :
> The corresponding Create in TMaskBase is not deprecated: is that by
design?

The Create in TMaskBase is never called directly by a user. He will get a
deprecated message from elsewhere.

Juha
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Tue, Oct 19, 2021 at 10:44 AM José Mejuto via lazarus
 wrote:

> Yes, at least it should. To completely disable the "[" syntax three
> options must be removed from default, "eMaskOpcodeOptionalChar",
> "eMaskOpcodeRange" and "eMaskOpcodeAnyCharOrNone".
>
> eMaskOpcodeAnyCharOrNone = [???] matches 0, 1, 2 or 3 chars.
> eMaskOpcodeRange = [a-z] matches 1 char from "a" to "z".
> eMaskOpcodeOptionalChar = [abc] matches "a" or "b" or "c"
>
> With the three options disabled "[" is interpreted as a regular
> character to be matched.
>
> With "eMaskOpcodeRange" and "eMaskOpcodeOptionalChar" enabled to match
> "a" or "-" or "z" the "-" must be escaped (something like regex) using
> the escapechar, by default "\", in this way "[a\-z]".

Thanks for explaining that.
Sounds to me like you should not enable both of them, unless you want
something to match 'a'..'f' or '+' or '-' [a-f+\-] ?

Lots of documentation to do.

With only eMaskOpcodeOptionalChar enabled [a-z] would match 'a', '-' or 'z'?
(This would be the old behaviour of TMask)

The eMaskOpcodeOptionalChar enum name is not very self-explanatory,
maybe we should come up with another name (better now than in half a
year).

And maybe a constant that has all opcodes except the ones that enable ranges.

-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[José Mejuto via lazarus]

El 18/10/2021 a las 22:39, Bart via lazarus escribió:


Most people are familiar with ranges "[a-z]", optional chars use the
same "[" syntax but without the dash "-", so "[abcde]" matches one
position with any of those chars or if you negate the set "[!abcde]" it
will match any char *except* any of those.


So, with that option specified ranges work like "sets" in the old
Masks implementation?
And [a-z] will mean either 'a' or '-' or 'z'?



Hello,

Yes, at least it should. To completely disable the "[" syntax three 
options must be removed from default, "eMaskOpcodeOptionalChar", 
"eMaskOpcodeRange" and "eMaskOpcodeAnyCharOrNone".


eMaskOpcodeAnyCharOrNone = [???] matches 0, 1, 2 or 3 chars.
eMaskOpcodeRange = [a-z] matches 1 char from "a" to "z".
eMaskOpcodeOptionalChar = [abc] matches "a" or "b" or "c"

With the three options disabled "[" is interpreted as a regular 
character to be matched.


With "eMaskOpcodeRange" and "eMaskOpcodeOptionalChar" enabled to match 
"a" or "-" or "z" the "-" must be escaped (something like regex) using 
the escapechar, by default "\", in this way "[a\-z]".


--

--
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Sun, Oct 17, 2021 at 7:37 PM José Mejuto via lazarus
 wrote:
> OpcodeOptionalChar (maybe the name should be OptionChar) works in the
> compiled stream as CheckMatch and if match go to next char; if not match
> continue checking without advance in the target string.
>
> Most people are familiar with ranges "[a-z]", optional chars use the
> same "[" syntax but without the dash "-", so "[abcde]" matches one
> position with any of those chars or if you negate the set "[!abcde]" it
> will match any char *except* any of those.

So, with that option specified ranges work like "sets" in the old
Masks implementation?
And [a-z] will mean either 'a' or '-' or 'z'?


-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Maxim Ganetsky via lazarus]

16.10.2021 15:13, Maxim Ganetsky via lazarus пишет:

16.10.2021 11:54, Juha Manninen via lazarus пишет:
On Sat, Oct 16, 2021 at 11:20 AM Juha Manninen 
mailto:juha.mannine...@gmail.com>> wrote:


    Yes, the TMaskWindows indeed behaves wrong. '**.pas*' matches with
    '*abc.pas.bak*'.


Fixed in a111270ed0. Please test.
No code changes were needed. I only disabled one of the Windows 
quirks, namely eWindowsQuirk_Extension3More.

Please read the comments in code for details.


Thanks, I'll test later and report.


Works OK now.

--
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] TMask revisited

[José Mejuto via lazarus]

El 17/10/2021 a las 9:08, Juha Manninen via lazarus escribió:



What does eMaskOpcodeOptionalChar do?
It's not very clear to me from the comments in the code.

Good question. It is the one syntax I don't fully understand yet.
Below is an explanation from José himself. I will study this syntax 
later and add more unit test cases.

---
eMaskOpcodeOptionalChar is the set "1[ab]2" matches "1a2" and "ab2" but 
not "1c2".

---
I am not sure if it is correct. I will return to this later...


Hello,

OpcodeOptionalChar (maybe the name should be OptionChar) works in the 
compiled stream as CheckMatch and if match go to next char; if not match 
continue checking without advance in the target string.


Most people are familiar with ranges "[a-z]", optional chars use the 
same "[" syntax but without the dash "-", so "[abcde]" matches one 
position with any of those chars or if you negate the set "[!abcde]" it 
will match any char *except* any of those.


Have a nice day.


--

--
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Juha Manninen via lazarus]

On Sun, Oct 17, 2021 at 1:10 AM Bart via lazarus <
lazarus@lists.lazarus-ide.org> wrote:

> I see you implemented the "old" constructors with the TMaskOptions
> parameter and deprecated them as suggested.
> Thanks for that.
> Maybe add: 'Will be removed in 2.4' or similar to the deprecated message?
> The corresponding Create in TMaskBase is not deprecated: is that by design?
>

Actually this will be published in 2.4 initially. After that the deprecated
stuff can be removed.
I added comments.


Thanks also for implementing MaskOpCodesDisableRange
> It needs to be apllied in the TShell* components, to presevere default
> behaviour.
>

ShellCtrls.pas was updated already in the initial commit d7036bb0009 last
wednesday.


The method TMask.MatchesWindowsMask still is not implemented, so it
> immediately causes compiler errors in my code.
> This also needs to be implemented an deprecated IMHO.
>

Ok, yes. I implemented it in c28cc988ec. It requires a
temporary TMaskUTF8Windows instance because the syntax is decided in
creation.


What does eMaskOpcodeOptionalChar do?
> It's not very clear to me from the comments in the code.
>

Good question. It is the one syntax I don't fully understand yet.
Below is an explanation from José himself. I will study this syntax later
and add more unit test cases.
---
eMaskOpcodeOptionalChar is the set "1[ab]2" matches "1a2" and "ab2" but not
"1c2".
---
I am not sure if it is correct. I will return to this later...

Juha
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Sat, Oct 16, 2021 at 11:20 PM Juha Manninen via lazarus
 wrote:

> Ideas? Comments?

I see you implemented the "old" constructors with the TMaskOptions
parameter and deprecated them as suggested.
Thanks for that.
Maybe add: 'Will be removed in 2.4' or similar to the deprecated message?
The corresponding Create in TMaskBase is not deprecated: is that by design?

Thanks also for implementing MaskOpCodesDisableRange
It needs to be apllied in the TShell* components, to presevere default
behaviour.

The method TMask.MatchesWindowsMask still is not implemented, so it
immediately causes compiler errors in my code.
This also needs to be implemented an deprecated IMHO.

What does eMaskOpcodeOptionalChar do?
It's not very clear to me from the comments in the code.

-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Juha Manninen via lazarus]

I turned the define RANGES_AUTOREVERSE into a run time option in
67111d9a30.
A boolean RangeAutoReverse is True by default. I don't really know when
somebody would want to turn it off.

I also added constructors and functions for legacy syntax, which means [?]
represents a literal '?' and characters cannot be escaped with '\'.
  constructor CreateLegacy() etc.
That was the syntax used earlier but the default syntax now is improved.
[?] is a wildcard and means 0 or 1 characters. Remember, '?' alone is 1
character.
[??] means 0,1 or 2 chars. Useful and quite intuitive.
Any character can be escaped with '\' now. For example literal '?' is \?,
and literal '*' is \*.
In the Windows mask syntax that is always disabled for obvious reasons.

Question is, should the old legacy syntax be used when default constructors
and functions are called? Like TMask.Create() etc.
Then the improved syntax would be supported by constructors having
different names.
Mask syntax has no standards. They have evolved and diversified. How does
Delphi's Mask work?
A positive fact however is that some 99.5% of masks use simple '?' and '*'
wildcards and continue to work in any case.

The TMask syntax can be tweaked in many ways. A set TMaskOpcodesSet can be
passed as a parameter to constructors / functions.
I made constants MaskOpCodesDisableRange which treats all brackets [..] as
literal characters
and MaskOpCodesNoEscape which is the "legacy" syntax explained above.
The Windows style mask can be tweaked with TWindowsQuirkSet.

Ideas? Comments?

Juha
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Sat, Oct 16, 2021 at 3:19 PM Maxim Ganetsky via lazarus
 wrote:

> > So far, this has not been reporduced by others in this thread.
>
> Juha already reproduced it. Happens in `Find in Files` dialog.

That's GMail for you.
When I wrote that, it was a reply to the only new message in this
thread (as per GMail).
Once I hit "Send" I saw numerous new messages, including the one where
it is confirmed.

Ahh, computers 

-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Maxim Ganetsky via lazarus]

16.10.2021 16:11, Bart via lazarus пишет:

On Sat, Oct 16, 2021 at 12:14 AM DougC via lazarus
 wrote:


Shouldn't the mask "*.pas*" be used to match file.pas.bak ? If so, the old code 
and new code are ok.


You miss the point.
It was said that the mask '*.pas;*.pp;*.inc' now also matched the file
foo.pas.bak, which would be a bug.
So far, this has not been reporduced by others in this thread.


Juha already reproduced it. Happens in `Find in Files` dialog.

--
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] TMask revisited

[Bart via lazarus]

On Sat, Oct 16, 2021 at 12:14 AM DougC via lazarus
 wrote:

> Shouldn't the mask "*.pas*" be used to match file.pas.bak ? If so, the old 
> code and new code are ok.

You miss the point.
It was said that the mask '*.pas;*.pp;*.inc' now also matched the file
foo.pas.bak, which would be a bug.
So far, this has not been reporduced by others in this thread.

-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Maxim Ganetsky via lazarus]

16.10.2021 13:00, Juha Manninen via lazarus пишет:

P.S.
Not related to the Mask code but I installed TortoiseGit on Windows.
It works OK but still does not offer anything as good as *gitk*.
The "Show log" view shows commits and their messages, and names of 
changed files but no diffs.

Getting diffs requires extra mouse clicking for each file separately.


You can right-click on the revision entry (in the log) itself, and there 
you will be able to open unified diff viewer. Also 'raw' diffs are not 
very convenient to me, I prefer to inspect them in a separate viewer 
(TortoiseGit has an excellent built-in one, and on Linux I use Meld).


Then there is "Revision graph" which is actually a branch graph. Not 
very useful.


Revision graph is located at the left of the log window. You mean it? It 
looks just like in any other tool?



Earlier I have tried SmartGit but I didn't see any benefit using it either.
"*git gui*", *gitk* + some git console commands work best for me.


You didn't try to cherry-pick, TortoiseGit has the best interface for 
this in my experience.


--
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] TMask revisited

[Maxim Ganetsky via lazarus]

16.10.2021 11:54, Juha Manninen via lazarus пишет:
On Sat, Oct 16, 2021 at 11:20 AM Juha Manninen 
mailto:juha.mannine...@gmail.com>> wrote:


Yes, the TMaskWindows indeed behaves wrong. '**.pas*' matches with
'*abc.pas.bak*'.


Fixed in a111270ed0. Please test.
No code changes were needed. I only disabled one of the Windows quirks, 
namely eWindowsQuirk_Extension3More.

Please read the comments in code for details.


Thanks, I'll test later and report.

--
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] TMask revisited

[Juha Manninen via lazarus]

On Sat, Oct 16, 2021 at 12:23 AM Bart via lazarus <
lazarus@lists.lazarus-ide.org> wrote:

> > Yes, there should be an option for backwards compatible syntax, like
> "[?]" . However the small changes in the syntax can be seen as improvements.
> > There already is a define for converting "[z-a]" to "[a-z]".
>
> Why a define?
> Shouldn't that better be configurable at runtime?
>

Yes, it should be a run time option. I will look at it.
BTW, I forgot the details of "[?]" syntax. I will go through the old mails.


>> José's code also catered for some odd DOS behaviour where a mask like
> >>
> >> 'file.txt?' actually matches with the filename 'file.txt' and a mask
> >> 'file?.txt' would match 'file1234.txt' (but not 'file12345.txt').
> >> That would be rather counter-intuitive to most users.
> >> I did not test (it was in one of the "quirks" settings) if it behaves
> >> like this by default, but that should IMHO not be the case.
>
> I tested it now, and by default it returns fals for those cases.
>

Bart, can you please list cases where the Windows mask behavior differs
from actual Windows cmd prompt behavior.
Adding them to the unit test case would help, too.
The case with Unit1.pas.bak syntax was fixed already.


I did not study th enre code at length, but it looks quite a bit more
> complicated to me than the old one.
> Not sure if I will be able to contribute to it anymore.
>

It is not so bad. Debugging is difficult because pointer syntax is used,
but I remember the old code was difficult to debug as well.
There are 2 essential methods:
 procedure Compile and function Matches().
I refactored the procedure Compile a bit. Originally it was one big block
of code.

Juha

P.S.
Not related to the Mask code but I installed TortoiseGit on Windows.
It works OK but still does not offer anything as good as *gitk*.
The "Show log" view shows commits and their messages, and names of changed
files but no diffs.
Getting diffs requires extra mouse clicking for each file separately.
Then there is "Revision graph" which is actually a branch graph. Not very
useful.
Earlier I have tried SmartGit but I didn't see any benefit using it either.
"*git gui*", *gitk* + some git console commands work best for me.
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Juha Manninen via lazarus]

On Sat, Oct 16, 2021 at 11:20 AM Juha Manninen 
wrote:

> Yes, the TMaskWindows indeed behaves wrong. '**.pas*' matches with '
> *abc.pas.bak*'.
>

Fixed in a111270ed0. Please test.
No code changes were needed. I only disabled one of the Windows quirks,
namely eWindowsQuirk_Extension3More.
Please read the comments in code for details.

Juha
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Juha Manninen via lazarus]

On Fri, Oct 15, 2021 at 3:38 PM Maxim Ganetsky via lazarus <
lazarus@lists.lazarus-ide.org> wrote:

> Please try with, for example bla.pas and bla.pas.bak files.
>

I got another mail from Maxim. It was also sent to this list but didn't
appear.
I also configured my Windows under Virtualbox for GitLab. Now I can test
with Windows and I can reproduce the issue.
Yes, the TMaskWindows indeed behaves wrong. '**.pas*' matches with '
*abc.pas.bak*'.
Now I added a test revealing it. I keep studying the issue...

Juha
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[DougC via lazarus]

Shouldn't the mask "*.pas*" be used to match file.pas.bak ? If so, the old code 
and new code are ok.



Doug C.






 On Fri, 15 Oct 2021 17:09:03 -0400 Bart via lazarus 
 wrote 


On Fri, Oct 15, 2021 at 2:38 PM Maxim Ganetsky via lazarus
 wrote:

> Please try with, for example bla.pas and bla.pas.bak files.

Mask: *.pas;*.pp;*.inc
Filename: file.pas.bak
Result: does not match, both in the old code and the new code.

-- 
Bart
-- 
___
lazarus mailing list
mailto:lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Fri, Oct 15, 2021 at 1:55 AM Juha Manninen via lazarus
 wrote:

>
> On Thu, Oct 14, 2021 at 7:57 PM Bart via lazarus 
>  wrote:
>>
>> You have changed the existing interface for both TMask and the
>> Matches(Windows)Mask(List) functions.
>> TMaskOptions has been removed.
>
>
> It can be added for compatibility.

Please do.
But, also feel free to deprecate them.

>> Noticable the ability to NOT interpret [] as a set in the mask has now
>> disappeared.
>
>
> How should a range / set [] be interpreted?

??
[az] is interpreted as either a or z by default
Diasbling use of sets would interpret this as literally [az] (4 chars).

> Now the same thing is done by passing MaskOpCodesDisableRange constant where 
> type TMaskOpcodesSet parameter is used.
> TMaskOptions can be restored for compatibility.

OK.

>> You have sacrificed consistent and reliable behaviour for gain of speed.
>
> No Bart, that is not true. This is not only about speed. The old Mask was 
> buggy and limited.

I agree about the limited part.
Not many bugs were filed against asks unit, and I did not encounter
them, but my locale is Ducth and the ocaasional non-ascii I happen to
have in my filenames all process correctly.

> Please run the test project testmasks.lpi. All current tests pass.
> Now checkout a revision before my Mask changes, eg. e5ed5082d and run tests 
> again. Many errors. Notably there was no support for Unicode.

I'll have a look.


> Yes, there should be an option for backwards compatible syntax, like "[?]" . 
> However the small changes in the syntax can be seen as improvements.
> There already is a define for converting "[z-a]" to "[a-z]".

Why a define?
Shouldn't that better be configurable at runtime?


>> José's code also catered for some odd DOS behaviour where a mask like
>>
>> 'file.txt?' actually matches with the filename 'file.txt' and a mask
>> 'file?.txt' would match 'file1234.txt' (but not 'file12345.txt').
>> That would be rather counter-intuitive to most users.
>> I did not test (it was in one of the "quirks" settings) if it behaves
>> like this by default, but that should IMHO not be the case.

I tested it now, and by default it returns fals for those cases.

> Yes it is counter-intuitive but it matches with Windows' behavior.
> You can test it on a DOS/Windows cmd prompt.

file?.txt matches file.txt and file1.txt, but not file12.txt, so ? as
last char before . is treated as no or 1 character.

> That is why it is called TMaskWindows. The normal TMask behaves more 
> logically.
> The "quirks" prove that Jose has tested it a lot.
>
> About the speed: TMask can also be used without disk file operations. Then 
> the speed matters.
Yes it can, but it was desigend for filemasks (see the original
parameter names).

> I will look at the compatibility issues and possible bugs tomorrow.

Thank you.

I did not study th enre code at length, but it looks quite a bit more
complicated to me than the old one.
Not sure if I will be able to contribute to it anymore.


-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Fri, Oct 15, 2021 at 2:38 PM Maxim Ganetsky via lazarus
 wrote:

> Please try with, for example bla.pas and bla.pas.bak files.

Mask: *.pas;*.pp;*.inc
Filename: file.pas.bak
Result: does not match, both in the old code and the new code.

-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Juha Manninen via lazarus]

On Fri, Oct 15, 2021 at 3:38 PM Maxim Ganetsky via lazarus <
lazarus@lists.lazarus-ide.org> wrote:

> Please try with, for example bla.pas and bla.pas.bak files.
>

I did basically that. I copied a .pas file to .pas.bak.
Find in Files does not find extra entries with File mask
"*.pas;*.pp;*.inc;*.lpr;*.lfm"
With File mask "*.pas;*.pp;*.inc;*.lpr;*.lfm;*.bak" they are correctly
found in the .bak file.
Works as expected.

Is the bug you experience caused by my recent unit Masks changes?
Does it depend on OS / Platform?

Juha
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Juha Manninen via lazarus]

On Fri, Oct 15, 2021 at 3:10 PM Sven Barth via lazarus <
lazarus@lists.lazarus-ide.org> wrote:

> Non-static class methods *do* have a Self parameter, but it's the class
> type and not the class instance. They can also be virtual, which is a
> really great feature of the Object Pascal language (I often miss that at
> work where I use C++). They are also assignment compatible to method
> variables ("of object").
>
> Static methods on the other hand don't have a Self parameter, can't be
> virtual and are assignment compatible to normal procedure variables.
>

Ok, right.
I confess I never used "static" with class procedures/functions. Thanks for
the info.

Juha
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Maxim Ganetsky via lazarus]

15.10.2021 02:07, Juha Manninen via lazarus пишет:
On Thu, Oct 14, 2021 at 12:53 AM Maxim Ganetsky via lazarus 
mailto:lazarus@lists.lazarus-ide.org>> 
wrote:


Don't know if it is related but just noticed that `Find in files`
dialog
now "ignores" file masks.

E. g. I have the following file mask: `*.pas;*.pp;*.inc;*.lpr;*.lfm`,
but it also finds (existing) entries in *.bak files, which is not
really
desired.


I cannot reproduce that. No matches in *.bak files here.


Please try with, for example bla.pas and bla.pas.bak files.

--
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] TMask revisited

[Sven Barth via lazarus]

Juha Manninen via lazarus  schrieb am Fr.,
15. Okt. 2021, 14:03:

> This may be a dummy question but what does "static" do for a class
> procedure?
> José's mask code has for example:
>
>   class procedure Exception_IncompleteMask(); static;
>
> I thought class procedures are kind of static anyways, meaning they don't
> have a Self pointer.
>

Non-static class methods *do* have a Self parameter, but it's the class
type and not the class instance. They can also be virtual, which is a
really great feature of the Object Pascal language (I often miss that at
work where I use C++). They are also assignment compatible to method
variables ("of object").

Static methods on the other hand don't have a Self parameter, can't be
virtual and are assignment compatible to normal procedure variables.

Regards,
Sven

>
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Juha Manninen via lazarus]

This may be a dummy question but what does "static" do for a class
procedure?
José's mask code has for example:

  class procedure Exception_IncompleteMask(); static;

I thought class procedures are kind of static anyways, meaning they don't
have a Self pointer.

Juha
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Juha Manninen via lazarus]

I restored the backwards compatible TMaskOptions in 04b0a04a45.
Please test.

When ranges (sets) are disabled, [] is interpreted as literal []. Works
perfectly.
Next I will study the little syntax differences. There should be a backwards
compatible option for those, too.

Juha
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Juha Manninen via lazarus]

On Fri, Oct 15, 2021 at 2:54 AM Juha Manninen 
wrote:

> Noticable the ability to NOT interpret [] as a set in the mask has now
>> disappeared.
>>
>
> How should a range / set [] be interpreted?
>

Ok, forget that question. I will study this more tomorrow.

Juha
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Juha Manninen via lazarus]

On Thu, Oct 14, 2021 at 7:57 PM Bart via lazarus <
lazarus@lists.lazarus-ide.org> wrote:

> You have changed the existing interface for both TMask and the
> Matches(Windows)Mask(List) functions.
> TMaskOptions has been removed.
>

It can be added for compatibility.


Noticable the ability to NOT interpret [] as a set in the mask has now
> disappeared.
>

How should a range / set [] be interpreted?

This is not some cornercase: Suppose you have files that have
> filenames like foo[1].dat, foo[2].dat etc.
> Now you could specify a mask of 'foo[*].dat' with the moDisableSets
> option set and it would find those files.
> The new implementation won't.
> (This was the usecase of the person who reported the bug for
> TShellListView, which first was hacked by using [[] to mean a single
> '[', but later was properly fixed by adding the TMaskOptions.
>

Now the same thing is done by passing MaskOpCodesDisableRange constant
where type TMaskOpcodesSet parameter is used.
TMaskOptions can be restored for compatibility.


This was discussed at length in february.
>
> Now you're doing it all over again.
> You have sacrificed consistent and reliable behaviour for gain of speed.
>

No Bart, that is not true. This is not *only* about speed. The old Mask was
buggy and limited.
Please run the test project testmasks.lpi. All current tests pass.
Now checkout a revision before my Mask changes, eg. e5ed5082d and run tests
again. Many errors. Notably there was no support for Unicode.

Yes, there should be an option for backwards compatible syntax, like
"[?]" . However the small changes in the syntax can be seen as improvements.
There already is a define for converting "[z-a]" to "[a-z]".


José's code also catered for some odd DOS behaviour where a mask like
>
'file.txt?' actually matches with the filename 'file.txt' and a mask
> 'file?.txt' would match 'file1234.txt' (but not 'file12345.txt').
> That would be rather counter-intuitive to most users.
> I did not test (it was in one of the "quirks" settings) if it behaves
> like this by default, but that should IMHO not be the case.
>

Yes it is counter-intuitive but it matches with Windows' behavior.
You can test it on a DOS/Windows cmd prompt.
That is why it is called TMaskWindows. The normal TMask behaves more
logically.
The "quirks" prove that Jose has tested it a lot.

About the speed: TMask can also be used without disk file operations. Then
the speed matters.
Disk operations are typically slow, yes.

I will look at the compatibility issues and possible bugs tomorrow.

Regards,
Juha
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Juha Manninen via lazarus]

On Thu, Oct 14, 2021 at 12:53 AM Maxim Ganetsky via lazarus <
lazarus@lists.lazarus-ide.org> wrote:

> Don't know if it is related but just noticed that `Find in files` dialog
> now "ignores" file masks.
>
> E. g. I have the following file mask: `*.pas;*.pp;*.inc;*.lpr;*.lfm`,
> but it also finds (existing) entries in *.bak files, which is not really
> desired.
>

I cannot reproduce that. No matches in *.bak files here.

Juha
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Thu, Oct 14, 2021 at 6:54 PM Bart  wrote:

> This was discussed at length in february.

From the discussion in february:

Feb 24 11:22 AM
> And also define if a compatibility break is a bug in the new code or in
> the old code. In example my mask supports (there is a define to disable)
> "[z-a]" converting it to "[a-z]" which is a compatibility break. Also
> there is the support (also can be disabled) for the mask "[?]" which is
> the counterpart for "*" but with one char position.

Current behaviour of sets and wildcards should not be changed by default.
E.g. TShellTreeView and TShellListView use the Masks unit to populate
the tree/view.
An option to have the behaviour you described would be OK, the
TMaskOption can be extended for that.



Feb 26 7:15 PM
The normal way of doing this is:
Deprecate the function in question, but do NOT kill it's functionality.
Add a useful deprecated message.
Remove the function in the next major release (deprecate in 2.1, and
so 2.2, only remove in 2.3, so it'll be gone in 2.4).
Simply removing functionality like you have done now will alienate
users from Lazarus, since apparently "we" cannot be trusted.

Juha: you seem to be obsessed with speeding up string handling code.
This is not really a problem as long as you are not deaf to arguments
against your changes.
You introduce new bugs, remove old features, all for the sake of speed.
-

Let me be very clear: I am NOT against improving and expanding TMask
and associated functions.
Overhauling the "matching algorithm" so that it is no longer O(n^3) or
O(n^4) is a good thing.
Speeding up string handling functions is most of the time also a good thing.

Breaking things in the long run also may be acceptable.
Breaking them over night, unannounced and especially __removing
existing functionality__ is a no-no from my point of view.


José's code also catered for some odd DOS behaviour where a mask like
'file.txt?' actually matches with the filename 'file.txt' and a mask
'file?.txt' would match 'file1234.txt' (but not 'file12345.txt').
That would be rather counter-intuitive to most users.
I did not test (it was in one of the "quirks" settings) if it behaves
like this by default, but that should IMHO not be the case.

-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Bart via lazarus]

On Wed, Oct 13, 2021 at 5:16 PM Juha Manninen via lazarus
 wrote:


> Please test everybody. I will read the old posts more carefully later.

You have changed the existing interface for both TMask and the
Matches(Windows)Mask(List) functions.
TMaskOptions has been removed.
Noticable the ability to NOT interpret [] as a set in the mask has now
disappeared.
From my test program:
FAIL
Filename: 'Heoworld', MaskList: 'He[lo]world',
Options=[moDisableSets], WindowsMask=FALSE
ResOld: FALSE, ResNew: TRUE

This is not some cornercase: Suppose you have files that have
filenames like foo[1].dat, foo[2].dat etc.
Now you could specify a mask of 'foo[*].dat' with the moDisableSets
option set and it would find those files.
The new implementation won't.
(This was the usecase of the person who reported the bug for
TShellListView, which first was hacked by using [[] to mean a single
'[', but later was properly fixed by adding the TMaskOptions.

This was discussed at length in february.

Now you're doing it all over again.
You have sacrificed consistent and reliable behaviour for gain of speed.

-- 
Bart
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Maxim Ganetsky via lazarus]

13.10.2021 18:16, Juha Manninen via lazarus пишет:
I restored the great TMask implementation by José Mejuto in our 
development branch.

I also fixed a few issues that hindered the transition last time.
It was discussed at least in this mailing list with title
  "unit Masks vs. unit FPMasks"
in 23 February this year.

The logic in ShellCtrls unit is retained as it was. MaskList is used 
only when there are many ';' separated masks.
I found a way to disable ranges in the new TMask code. The syntax 
differs a little from the old one though. If the old TMaskOptions syntax 
is needed, it can be added as a compatibility layer.


Don't know if it is related but just noticed that `Find in files` dialog 
now "ignores" file masks.


E. g. I have the following file mask: `*.pas;*.pp;*.inc;*.lpr;*.lfm`, 
but it also finds (existing) entries in *.bak files, which is not really 
desired.



function TMaskList.MatchesWindowsMask() now works using a hack.
It is deprecated for that reason.

Tests to lazutils/test/testmasks.lpr were added for disabled ranges.

Please test everybody. I will read the old posts more carefully later.


--
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] TMask revisited

[Marcus Sackrow via lazarus]

Hi,

Am 13.10.21 um 18:28 schrieb Juha Manninen via lazarus:

On Wed, Oct 13, 2021 at 7:03 PM Marcus Sackrow via lazarus
mailto:lazarus@lists.lazarus-ide.org>>
wrote:

it breaks "make all" for x86_64-linux


LazBuild, yes. Again I forgot to test compiling it.
Now it is fixed. Please test.



Thanks, compiles again.


Greetings,

Marcus

-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Juha Manninen via lazarus]

On Wed, Oct 13, 2021 at 7:03 PM Marcus Sackrow via lazarus <
lazarus@lists.lazarus-ide.org> wrote:

> it breaks "make all" for x86_64-linux
>

LazBuild, yes. Again I forgot to test compiling it.
Now it is fixed. Please test.

Regards,
Juha
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

Re: [Lazarus] TMask revisited

[Marcus Sackrow via lazarus]

Hi,

Am 13.10.21 um 17:16 schrieb Juha Manninen via lazarus:

I restored the great TMask implementation by José Mejuto in our
development branch.
I also fixed a few issues that hindered the transition last time.
It was discussed at least in this mailing list with title
 "unit Masks vs. unit FPMasks"
in 23 February this year.



it breaks "make all" for x86_64-linux

https://build.alb42.de:8081/job/LCL_off_x86_64-linux/9078/console



Hint: (5091) Local variable "InheritedOptionStrings" of a managed type
does not seem to be initialized

Error: (4025) Incompatible type for arg no. 2: Got "Array Of Const",
expected "Boolean"
:masks.pas(816,23)
Hint: (5039) Found declaration: constructor Create(const
RawByteString;Boolean=`FALSE`;TMaskOpcodesSet=``);
lazbuild.lpr(1880) Fatal: (10026) There were 1 errors compiling module,
stopping
Fatal: (1018) Compilation aborted
Makefile:4673: recipe for target 'lazbuild' failed
make[2]: *** [lazbuild] Error 1


Could you please look into?



Greetings,

Marcus

--
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus

[Lazarus] TMask revisited

[Juha Manninen via lazarus]

I restored the great TMask implementation by José Mejuto in our development
branch.
I also fixed a few issues that hindered the transition last time.
It was discussed at least in this mailing list with title
 "unit Masks vs. unit FPMasks"
in 23 February this year.

The logic in ShellCtrls unit is retained as it was. MaskList is used only
when there are many ';' separated masks.
I found a way to disable ranges in the new TMask code. The syntax differs a
little from the old one though. If the old TMaskOptions syntax is needed,
it can be added as a compatibility layer.

function TMaskList.MatchesWindowsMask() now works using a hack.
It is deprecated for that reason.

Tests to lazutils/test/testmasks.lpr were added for disabled ranges.

Please test everybody. I will read the old posts more carefully later.

Regards,
Juha
-- 
___
lazarus mailing list
lazarus@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus