Incorrect SetFoldLevel signature

[Ekopalypse]

trafficstars

SetFoldLevel needs to have an integer type as its second parameter instead of the FoldLevel enum.

[mahee96]

@Ekopalypse , There are auto generated enums in GatewayDomain.cs file, this was intentional to have more meaning to the flag grouping.

public enum FoldFlag
        {
            LINEBEFORE_EXPANDED = 0x0002,
            LINEBEFORE_CONTRACTED = 0x0004,
            LINEAFTER_EXPANDED = 0x0008,
            LINEAFTER_CONTRACTED = 0x0010,
            LEVELNUMBERS = 0x0040,
            LINESTATE = 0x0080
        }

Why don't you try using it instead, if you still face issue re-open this issue.

If you strongly feel it should be int/uint as per the official interface doc of Scintilla, then please open a new issue as question because it covers more fields than the FOLDFLAG alone and needs to be dealt separately.

[Ekopalypse]

@mahee96 But FoldFlag enum is something else and is used by the call SCI_SETFOLDFLAGS. SCI_SETFOLDLEVEL sets the current level per line and is used to determine which rows must be must be collapsed/expanded, while FoldFlags describe how folded/expanded rows should be visually displayed.

[mahee96]

Oops sorry for the confusion, I meant to refer you to FoldLevel enum in the Gatewaydomain.cs, Could you please check that enum satisfies your request

[mahee96]

@Ekopalypse checkout this

/// <summary>The number of display lines needed to wrap a document line (Scintilla feature SC_FOLDLEVEL)</summary> 
public enum FoldLevel { 
     BASE = 0x400, 
     WHITEFLAG = 0x1000, 
     HEADERFLAG = 0x2000, 
     NUMBERMASK = 0x0FFF 
}

[Ekopalypse]

No, these are the flags which need to be used in combination with additional integers. Let me give you an example, assume in the following code and we want to mark the { as the open folding tag and } as the closing tag like npp does.

        public enum FoldLevel
        {
            BASE = 0x400,
            WHITEFLAG = 0x1000,
            HEADERFLAG = 0x2000,
            NUMBERMASK = 0x0FFF
        }
        

To set the respective flags you would set to line 0 -> BASE 1 -> BASE | HEADERFLAG 2 - 6 -> BASE + 1 7 -> BASE

Now scintilla knows where to put the folding markers and how many lines to collapse or fold.

[Ekopalypse]

Maybe this example is more descriptive

public
{
	{
		{
			value
		}
	}
}

The lines need to be set like 0x0400 0x2400 0x2401 0x2402 0x0403 0x0403 0x0402 0x0401 0x0400

[mahee96]

okay there is a slight discrepancy in Scintilla.iface file and the official documentation which we have to fix.

The documentation states:

SCI_SETFOLDLEVEL(line line, int level)
SCI_GETFOLDLEVEL(line line) → int

whereas the Scintilla.iface file states:

# Set the fold level of a line.
# This encodes an integer level along with flags indicating whether the
# line is a header and whether it is effectively white space.
set void SetFoldLevel=2222(line line, FoldLevel level)

# Retrieve the fold level of a line.
get FoldLevel GetFoldLevel=2223(line line,)

I think while updating the converter, we missed some info which states the obvious for types and enumerations.

So essentially those enumerations if in C/C++ would be not typed but plain integers which could be combined using plain pipe/or operation. But in C# it needs to be of int type as the document states.

[mahee96]

@Ekopalypse , a quick fix can be made for this particular field for now, but the inventory of affected fields will be performed later some time and a fix will be given.

[mahee96]

@Ekopalypse, The fix is created as a pull request, review the commit and see if the issue will be resolved after this merge.

Thanks.

[Ekopalypse]

@mahee96 - just to be clear, I am a C# noob. This is my very first C# project, so my knowledge of C# is actually .... non-existent. The PR makes an int out of the enums and, yes, that's what I need. However, according to my research, the C# int type is a 32bit signed integer and thus is actually wrong, since Scintilla defines positions and lines as IntPtr. But of course I can understand if someone argues that it makes little or no sense to use files with 2000 million lines, but on the other hand it may be that someone wants to write a C# plugin to scan large data and possibly edit sections in Npp. So whatever is decided is fine with me.

[kbilsted]

I agree, we should use an enum

[mahee96]

@Ekopalypse, That's great finding, I for once, forgot about the .NET runtime being managed and the standard int size(32-bit signed) is limiting when the actual data is a pointer type value.

In unmanaged code the int datatype is 32-bit signed on both x86/x64 but a Pointer type is dependent on the addressability ie, 32-bit unsigned on x86, 64-bit unsigned on x64. In managed code since pointers are unsafe and not recommended a type safe IntPtr type represents the equivalent of the pointer type as in unmanaged code.

IntPtr size is machine dependent, also it is a signed integer containers which can hold unsigned pointers from unmanaged code.

So as you said, the IntPtr is the appropriate one to be used here, because even scintilla document states the same.

This should be the reason for the issue #68 where the type of the underlying marshalled call to C++ application code returns a pointer type of 8bytes on x64 which doesn't fit on the 32-bit signed integer and overflowed data is lost.

Long cannot be used since it would essentially work for 64 bit space but in 32-bit space it would again cause the same issue where the structures/layout no longer maps to the underlying data structure in memory. ex: struct A{ int* var; int data; }; is not same as struct A{ long var; int data; } on 32-bit machine but is same in 64 bit machine when using long as a container type for pointer.

This is because in 64 bit space the struct size would be 8(int*)+4(int) = 12 bytes but on 32-bit space it will be 4(int*)+4(int) = 8 bytes. Hence this data when interpreted in managed code based on 8(long)+4(int) will always map to 12 bytes and will break the 32-bit plugin.

Reverse happens when using struct A{ int var; int data; } where 32-bit space plugins will work fine and 64-bit space plugins will break.

So using a IntPtr should take care of automatic pointer size space allocation in structs which should then provide correct mappings of layouts.

The other part to using IntPtr would be the pointer arithmetic, there is no direct pointer arithmetic such as +,- etc when dealing with IntPtr. static methods such as IntPtr.Add(ptr, off)...etc is present to handle these situations, but is not sufficient. We need to manually write the wrapper classes which perform pointer arithmetic if in case. ex:

public class Position : IEquatable<Position>
    {
        private readonly long pos;

        public Position(IntPtr pos)
        {
            this.pos = IntPtr.Size == sizeof(Int32)? pos.ToInt32(): pos.ToInt64();
        }

        public static Position operator +(Position a, Position b)
        {
           
            return new Position(IntPtr.Size == sizeof(Int32)? new IntPtr((int)(a.pos+b.pos)): new IntPtr(a.pos+b.pos));
        }

        public static Position operator -(Position a, Position b)
        {
            return new Position(IntPtr.Size == sizeof(Int32)? new IntPtr((int)(a.pos-b.pos)): new IntPtr(a.pos-b.pos));
        }
. . . . . . . . . .

[mahee96]

@kbilsted, I don't think its about enum anymore, its about the underlying struct mappings that are now causing problems due to mismatch in the types to be used, ex: using int is incorrect when scintilla says to use intptr_t whose equivalent in C# is IntPtr.

Let me know your thoughts on these.

[mahee96]

+1 using IntPtr would only increase burden on the developer who is using this template, since for arithmetic they have to perform cast to long/ulong/int/uint (considering fact that pointers are unsigned) even for 32-bit (putting 4 byte pointers into 8-byte containers doesn't hurt).

[mahee96]

Note for int, position, line scintilla states it uses intptr_t whose equivalent in C# should be IntPtr

[Ekopalypse]

Just for my understanding, there are no open issues that concern me, meaning where I should/can help or provide information? Please do not misunderstand this post, no pressure should be built. I am aware that this is an open source project and everyone here sacrifices their free time to provide others with a helpful tool. Thank you very much for that. So if there is something where I can support, with my non-existent C# knowledge, then bring it on.

[guidomarcel]

Changing the struct as @mahee96 did in the development branch solved one of my issues. We could merge the development branch into the master branch. What do you think @kbilsted ?

[guidomarcel]

@kbilsted , @mahee96 one more comment. I tested the NotepadPlusPlusPluginPack. in VS2022 Community Edition succesfully

[mahee96]

we are aware of this issue and will fix if it resurfaces to prevent regression.

Closing issue.