Recent

Author Topic: New ID attributes on SynEditHighLighter unit.  (Read 46096 times)

Edson

  • Hero Member
  • *****
  • Posts: 1044
New ID attributes on SynEditHighLighter unit.
« on: August 22, 2013, 11:40:47 pm »
Is it posible to include new ID attributes (and his correspond Property) on SynEditHighLighter?
Currently, i just can see:

  SYN_ATTR_COMMENT           =   0;
  SYN_ATTR_IDENTIFIER        =   1;
  SYN_ATTR_KEYWORD           =   2;
  SYN_ATTR_STRING            =   3;
  SYN_ATTR_WHITESPACE        =   4;
  SYN_ATTR_SYMBOL            =   5;                                             //mh 2001-09-13

I think there should be additionally: SYN_ATTR_NUMBER and SYN_ATTR_MACRO.

Lazarus 1.6 - FPC 3.0.0 - x86_64-win64 on  Windows 7

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5635
    • wiki
Re: New ID attributes on SynEditHighLighter unit.
« Reply #1 on: August 23, 2013, 12:15:46 am »
SYN_ATTR_NUMBER  makes sense.

MACRO ? How are you plan to use this.  Does defining it for *ALL* highlighters have an advance, even very few, if any will have it?

You only need this, if you do something like
Code: [Select]
for i := 0 to list.count do begin
  HighlighterOfUnknownClass := list[i];
  HighlighterOfUnknownClass.GehtDefaultAttribute(SYN_ATTR_NUMBER )
 
 

Or am I missing anything?

------
SYN_ATTR_NUMBER   I will add, IF you supply a patch.

The patch should include adding SYN_ATTR_NUMBER  to the implementation of GetDefaultAttribute of the majority of those highlighters that can return it.


Adding SYN_ATTR_NUMBER , without extending the GetDefaultAttribute will mean that someone will probably complain of it as a bug. And then I am the one who has he work to do.....

-----
 SYN_ATTR_MACRO  How many highlighters do we have, that have this, or what is a good example for how to use it.

If you write your own HL, you can always define it there.

we can add  SYN_ATTR_FIRST_FREE = 6

and then you can do
 SYN_ATTR_MACRO = SYN_ATTR_FIRST_FREE +1;

SYN_ATTR_FIRST_FREE can be changed if  other values are added.
Or in can be
SYN_ATTR_FIRST_FREE = 1000;


Edson

  • Hero Member
  • *****
  • Posts: 1044
Re: New ID attributes on SynEditHighLighter unit.
« Reply #2 on: August 23, 2013, 08:22:41 pm »
Thanks Martin for the information.

For "macro" I mean, the directive #define (of C), or the $IFDEF (of Pascal). I don't know if they have another name. I see it's called "fDirecAttri" in the C++ highlighter.

I realize that many highlighters redefine the properties:

    property CommentAttribute:
    property IdentifierAttribute:
    property KeywordAttribute:
    property StringAttribute:
    property SymbolAttribute:
    property WhitespaceAttribute:

and add some more, especially "NumberAttribute".

I think they should, use the propertys defined on "TSynCustomHighlighter", instead of re-declare their own (that is what I do), and grow innecesary. But for do that, we need more constants, like SYN_ATTR_NUMBER (and his property of course). FAIK, it will be more efficient.

Am I wrong?

I know,  I can always, modify my code for manage that, without modify "TSynCustomHighlighter", but it will better if it will be included.

If I have to do a patch. No problem. Just give me some information about that, and I would do.


Greettings.
Lazarus 1.6 - FPC 3.0.0 - x86_64-win64 on  Windows 7

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5635
    • wiki
Re: New ID attributes on SynEditHighLighter unit.
« Reply #3 on: August 23, 2013, 08:53:41 pm »
For "macro" I mean, the directive #define (of C), or the $IFDEF (of Pascal). I don't know if they have another name. I see it's called "fDirecAttri" in the C++ highlighter.

I would not use MACRO for #define, because there also are others (#include). In C they are called preprocessor directives. In Pascal there is no preprocessor, so they are just directives.

Quote
I realize that many highlighters redefine the properties:
    property CommentAttribute:
    property IdentifierAttribute:
...
Quote
and add some more, especially "NumberAttribute".

I think they should, use the propertys defined on "TSynCustomHighlighter", instead of re-declare their own (that is what I do), and grow innecesary. But for do that, we need more constants, like SYN_ATTR_NUMBER (and his property of course). FAIK, it will be more efficient.

Ok, so you do not just want to add the constants, but also add the properties.

As for growing:
adding
Code: [Select]
    property StringAttri: TSynHighlighterAttributes read fStringAttri;
produces less code and data, than having to add code to "GetDefaultAttribute"

Yet adding a property "DirectiveAttribute" to the base class (even if not published, and not visible in ObjectInspector) makes this property present and accessible in all classes, even if they do not use it. It will also show in code-completion, which is not desirable.


Besides, that this is not about saving a few bytes. This must be about design. If there was a good reason to be able to iterate all highlighter classes, and ask each of them for a certain attribute, then this makes sense. But for that you do NOT need the property. You only need the constant and access to "GetDefaultAttribute" (ok, it is protected so that would be an issue).
Actually for that there is
Code: [Select]
    property AttrCount: integer read GetAttribCount;
    property Attribute[idx: integer]: TSynHighlighterAttributes read GetAttribute;
and you can check Attr.Name = SYNS_AttrNumber / not as nice as having a number.....

------------
Anyway the conclusion is, we can talk about ways to iterate attributes, or ask for specific types by the use of ONE single function like "GetDefaultAttribute"

But I do not think, that it is a good idea to add further attibute properties to the base class (as far as I go, I would rather remove them, but don't worry, I wont, as it would break compatibility)


Edson

  • Hero Member
  • *****
  • Posts: 1044
Re: New ID attributes on SynEditHighLighter unit.
« Reply #4 on: August 25, 2013, 03:31:41 am »
Thanks Martin for the response.

I understand your point. But, would you, please tell me:
What is the reason of we have these properties:     

property CommentAttribute
property IdentifierAttribute
property KeywordAttribute
property StringAttribute
property SymbolAttribute
property WhitespaceAttribute

defined in "TSynCustomHighlighter" if we have to re-define them in each descendant class we use?
« Last Edit: August 25, 2013, 06:19:25 am by Edson »
Lazarus 1.6 - FPC 3.0.0 - x86_64-win64 on  Windows 7

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5635
    • wiki
Re: New ID attributes on SynEditHighLighter unit.
« Reply #5 on: August 25, 2013, 11:15:16 am »
I dont know why they where added. they existed before I joined. Probably even in the original synedit.
I can only guess that someone assumed they would be used by (almost) every HL ?


And you do not have to redefine them?. Though you do have to publish them (but thats a different topic).

Edson

  • Hero Member
  • *****
  • Posts: 1044
Re: New ID attributes on SynEditHighLighter unit.
« Reply #6 on: August 26, 2013, 05:38:49 pm »
I have checked the original SynEdt and these properties exist in it. I can also see, that it has one less property than the Lazarus versión, so I can deduct that someone added one more constant (and property) to "TSynCustomHighlighter" in the Lazarus version (probably in the indicated date):
  SYN_ATTR_SYMBOL   =   5;                 //mh 2001-09-13

I think the "TSynCustomHighlighter" class, have a good design. His properties "xxxxxAtribute" are usefull.
They are a elegant way to access to the attributes of some general categories, through the editor, independenty of wich highlighter is the editor using (Using "GetDefaultAttribute" is not elegant and not accesible).

I have a example. If you have one App with many editors windows opened, and each one can have one diferente highlighter. If you have to configure the attributes of one editor, it will be easy to use just one unique "configuration dialog" that access to the "highlighter" property of  the editor and using this general properties (the no used must return NIL) instead of ask wich highlighter is using this particular editor.

The example should be valid too for one editor with many possible highlighters.

It's not exactly iterates highlighters. It is a like generic access to highlighter attributes of any editor.
Lazarus 1.6 - FPC 3.0.0 - x86_64-win64 on  Windows 7

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5635
    • wiki
Re: New ID attributes on SynEditHighLighter unit.
« Reply #7 on: August 26, 2013, 07:41:15 pm »
I think the "TSynCustomHighlighter" class, have a good design. His properties "xxxxxAtribute" are usefull.
They are a elegant way to access to the attributes of some general categories, through the editor, independenty of wich highlighter is the editor using (Using "GetDefaultAttribute" is not elegant and not accesible).

I still prefer the  "GetDefaultAttribute"

It does not have to be a function. It can be ONE property

Code: [Select]
type TSynAttrId = Integer;

property AttributeByID [Id: TSynAttrId]: TSynHlAttr read ... write ...
 function HasAttrId(Id: TSynAttrId): boolean;
function AttrIdCount: Integer;
function AttrIdFromIndex(idx: Integer): TSynAttrId
property AttributeByIndex [Id: TSynAttrId] :TSynHlAttr read ... write ...
........


much more powerful , and no properties that do not work in child classes

Quote
I have a example. If you have one App with many editors windows opened, and each one can have one diferente highlighter. If you have to configure the attributes of one editor, it will be easy to use just one unique "configuration dialog" that access to the "highlighter" property of  the editor and using this general properties (the no used must return NIL) instead of ask wich highlighter is using this particular editor.

The example should be valid too for one editor with many possible highlighters.

It's not exactly iterates highlighters. It is a like generic access to highlighter attributes of any editor.

an editor such as the IDE itself.

highlighters already have a list of all attr. the ide uses that.



Edson

  • Hero Member
  • *****
  • Posts: 1044
Re: New ID attributes on SynEditHighLighter unit.
« Reply #8 on: November 22, 2013, 05:37:47 pm »
@Martin,

After some analysis, I still think it's necessary add, at least:

SYN_ATTR_NUMBER

to the "TSynCustomHighlighter".

Could be useful to include SYN_ATTR_DIRECTIVE, SYN_ATTR_FUNCTION, SYN_ATTR_VARIABLE.

Make public "GetDefaultAttribute", could help.
Lazarus 1.6 - FPC 3.0.0 - x86_64-win64 on  Windows 7

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5635
    • wiki
Re: New ID attributes on SynEditHighLighter unit.
« Reply #9 on: November 22, 2013, 05:46:59 pm »
I already wrote

Quote
SYN_ATTR_NUMBER   I will add, IF you supply a patch.

The patch should include adding SYN_ATTR_NUMBER  to the implementation of GetDefaultAttribute of the majority of those highlighters that can return it.


Adding SYN_ATTR_NUMBER , without extending the GetDefaultAttribute will mean that someone will probably complain of it as a bug. And then I am the one who has he work to do.....

Note, that I will only add it, if the patch also fixes the majority of HL to return it.

And it is only for GetDefaultAttr. No new global property

Edson

  • Hero Member
  • *****
  • Posts: 1044
Re: New ID attributes on SynEditHighLighter unit.
« Reply #10 on: November 22, 2013, 06:23:23 pm »
Ah OK, so the proposal is still opened. I just need to do a patch.

But I don't understand why I need to modify other HL.

For all I can see, it just need to include this lines to the unit SynEditHighlighter:

Code: [Select]
  SYN_ATTR_NUMBER            =   6;

...

  TSynCustomHighlighter = class(TComponent)
  ...
    property NumberAttribute: TSynHighlighterAttributes
      index SYN_ATTR_NUMBER read GetDefaultAttribute;
...

This won't affect others HL. All of them who use "NumberAttribute" property, have their own definition for this property.
Lazarus 1.6 - FPC 3.0.0 - x86_64-win64 on  Windows 7

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5635
    • wiki
Re: New ID attributes on SynEditHighLighter unit.
« Reply #11 on: November 22, 2013, 06:36:40 pm »
Again, I will NOT add a new property called NumberAttribute.

I will add the constan, IF:
... as already stated.

Please look at GetDefaultAttribute for eact highlighter.

Each highlighter  knows the constants. If I add a constant, then each HL, that has a number, must return for that constant....

-----------------------------------
The other way is

iterate all Attributes, using the existing
    property AttrCount: integer read GetAttribCount;
    property Attribute[idx: integer]: TSynHighlighterAttributes
      read GetAttribute;

and add
property AttributeUsageId read ;
to
TSynHighlighterAttributes


then you can find them by iterate. But all HL, must create the attribute with the ID
« Last Edit: November 22, 2013, 06:41:28 pm by Martin_fr »

Edson

  • Hero Member
  • *****
  • Posts: 1044
Re: New ID attributes on SynEditHighLighter unit.
« Reply #12 on: November 22, 2013, 07:22:16 pm »
Again, I will NOT add a new property called NumberAttribute.

I don't end to understand, but I suppose you have good reasons for that.

So you accept to add the constant and make public GetDefaultAttribute. It'¡s OK?

Each highlighter  knows the constants. If I add a constant, then each HL, that has a number, must return for that constant....

OK, I get your point. You want that if we add a new constant, all the HL can now return a correct attribute value, if we use GetDefaultAttribute() with this new constant.

Well I see this is not so necessary, based on that, now users, don't use GetDefaultAttribute() for access to NumberAttribute. But it could be included on the patch.

The other way for access attribute (AttrCount, Attribute[]), is not so secure as I can see, for identify a particular attribute.
Lazarus 1.6 - FPC 3.0.0 - x86_64-win64 on  Windows 7

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5635
    • wiki
Re: New ID attributes on SynEditHighLighter unit.
« Reply #13 on: November 22, 2013, 07:34:53 pm »
So you accept to add the constant and make public GetDefaultAttribute. It'¡s OK?

Yes.
And we can also add a constant for CompilerDirectives in that case.

We should introduce a new (better named) public method, that calls GetDefaultAttribute
Maybe
  function AttibuteForTokenType

It does NOT need to be virtual, and can just call GetDefaultAttribute.

Quote
OK, I get your point. You want that if we add a new constant, all the HL can now return a correct attribute value, if we use GetDefaultAttribute() with this new constant.

Well I see this is not so necessary, based on that, now users, don't use GetDefaultAttribute() for access to NumberAttribute. But it could be included on the patch.

The problem is not what it would break now, but:
once the method exists, and the constant exists, users will try to use it.

And then there will be bug report: The function returns for comments, but not for numbers, that is not consistent.
So I try to avoid that.


Quote
The other way for access attribute (AttrCount, Attribute[]), is not so secure as I can see, for identify a particular attribute.

How is it any less secure? In both cases you may end up with nil or an attribute.

But never mind, both are ok.

Edson

  • Hero Member
  • *****
  • Posts: 1044
Re: New ID attributes on SynEditHighLighter unit.
« Reply #14 on: November 22, 2013, 10:09:03 pm »
'AttibuteForTokenType', it's a good name for me.

If we are going to modify many HL, maybe we should consider to add other constants.

Quote
And we can also add a constant for CompilerDirectives in that case.

What it would be for?


Lazarus 1.6 - FPC 3.0.0 - x86_64-win64 on  Windows 7