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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211013/15857463/attachment.html>

Hi,

it breaks "make all" for x86_64-linux

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

<https://build.alb42.de:8081/job/LCL_off_x86_64-linux/ws/ide/lazbuild.lpr(629,97)> Hint: (5091) Local variable "InheritedOptionStrings" of a managed type does not seem to be initialized
<https://build.alb42.de:8081/job/LCL_off_x86_64-linux/ws/ide/lazbuild.lpr(931,50)> Error: (4025) Incompatible type for arg no. 2: Got "Array Of Const", expected "Boolean"
<https://build.alb42.de:8081/job/LCL_off_x86_64-linux/ws/components/lazutils/lib/x86_64-linux/masks.ppu>:masks.pas(816,23) Hint: (5039) Found declaration: constructor Create(const RawByteString;Boolean=`FALSE`;TMaskOpcodesSet=`<set>`); 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

On Wed, Oct 13, 2021 at 7:03 PM Marcus Sackrow via lazarus <

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211013/c79fcf99/attachment.html>

Hi,

Thanks, compiles again.

Greetings,

Marcus

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211013/e524c838/attachment.html>

13.10.2021 18:16, Juha Manninen via lazarus пишет:

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.

PM Juha Manninen 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.

PM Bart wrote

This was discussed at length in february. From the discussion in february:

Feb 24 11:22 AM

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.

On Thu, Oct 14, 2021 at 12:53 AM Maxim Ganetsky via lazarus <

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

Juha
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211015/f6f94a48/attachment.html>

On Thu, Oct 14, 2021 at 7:57 PM Bart via lazarus <

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

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

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211015/69960f70/attachment.html>

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

Juha
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211015/2ec04961/attachment-0001.html>

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211015/ba9cec4c/attachment.html>

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211015/6a977b2a/attachment.html>

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

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211015/dd2e2b37/attachment.html>

15.10.2021 02:07, Juha Manninen via lazarus пишет:

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

On Fri, Oct 15, 2021 at 3:10 PM Sven Barth via lazarus <

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

Juha
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211015/97df0b7a/attachment.html>

On Fri, Oct 15, 2021 at 3:38 PM Maxim Ganetsky via lazarus <

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211015/70f2bc0c/attachment-0001.html>

PM Maxim Ganetsky 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.

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

??
[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?

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.

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

PM Maxim Ganetsky 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.

On Fri, Oct 15, 2021 at 3:38 PM Maxim Ganetsky via lazarus <

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211016/af067ea2/attachment.html>

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211016/4aa9e1bb/attachment.html>

On Sat, Oct 16, 2021 at 12:23 AM Bart via lazarus <

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

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. -------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211016/94ca629b/attachment.html>

16.10.2021 11:54, Juha Manninen via lazarus пишет:

Thanks, I'll test later and report.

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

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.

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

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

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

PM Maxim Ganetsky 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 ....

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211017/d1bd48b4/attachment.html>

PM Juha Manninen 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.

On Sun, Oct 17, 2021 at 1:10 AM Bart via lazarus <

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211017/13706b9a/attachment.html>

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.

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

Works OK now.

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

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.

On Tue, Oct 19, 2021 at 5:34 PM Bart via lazarus <

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211019/6c80d47e/attachment.html>

Bart wrote

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.

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

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.

PM Juha Manninen 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.

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.

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.

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

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.

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.

On Thu, Oct 21, 2021 at 11:30 AM José Mejuto via lazarus <

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211021/62911700/attachment.html>

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)?

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!

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.

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

Makes sense.

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

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.

Bart wrote

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.

PM José Mejuto 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.

I've given it some more thought.

AFAICS the TMaskOpCode and TMaskOpcodesEnum types are not 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).

On Sat, Oct 23, 2021 at 11:21 PM Bart via lazarus <

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211023/3f437063/attachment.html>

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.

AM José Mejuto 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?

PM Juha Manninen 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

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?

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.

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.

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

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

On Mon, Oct 25, 2021 at 7:20 PM Bart via lazarus <

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211026/6fe0bdf1/attachment.html>

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 '-'?

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, at lMask[j]);
                       lLast:=TMaskOpCode.OptionalChar;
                     end else begin
                       Add(TMaskOpCode.Literal);
                       Add(lCPLength, at lMask[j]);
                       inc(cMatchMinimumLiteralBytes,lCPLength);
                       if
cMatchMaximumLiteralBytes<High(cMatchMaximumLiteralBytes) then
inc(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.

PM José Mejuto 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.

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,

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.

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

PM José Mejuto 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,

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

PM José Mejuto 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

Hello,

Line 780, current:

       Add(TMaskParsedCode.OptionalChar);
       Add(fCPLength, at fMask[fMaskInd]);
       fLastOC:=TMaskParsedCode.OptionalChar;

Line 780, new:

       if (mocSet in fMaskOpcodesAllowed) then begin
         Add(TMaskParsedCode.OptionalChar);
         Add(fCPLength, at 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.

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] :-)