notepad-plus-plus / nppPluginList

The official collection of Notepad++ plugins.
1.09k stars 357 forks source link

Something missing for ARM build in resource.h ? #310

Closed MarkusBodensee closed 3 years ago

MarkusBodensee commented 3 years ago

@chcg @donho

Sorry for the inconvenience and if this is not a real problem. I am not able to test the ARM build, since I have no such device.

I just browsed through the code and found these lines (file: resource.h, line: 31 and below). I was wondering if something for ARM build must be added there?

https://github.com/notepad-plus-plus/nppPluginList/blob/a9b71e3473a09103f09bb815ddc7cbafa8f3b320/src/resource.h#L31

donho commented 3 years ago

The symbol _WIN64 includes ARM64 already.

MarkusBodensee commented 3 years ago

@donho Thank you for your reply.

What I meant with "below" is, that this in turn results in not having a define like: #define PLJSON "pl.arm64.json" present which is used in nppPluginList.rc file.

But looks like this does not have any negative effect in this case. But just wanted to mention to be on safe side. Thank you.

chcg commented 3 years ago

@donho The finding from @MarkusBodensee is valid. For the arm64 dll the json for x64 is currently used, if you do no magic on your local machine. This could be seen in the build folder within the nppPluginList.res file. I'm just wondering how this seems to work during testing of the arm64 build as plugin download was already tested there.

MarkusBodensee commented 3 years ago

Sorry @donho, I should have added the details of my second post directly to the initial one. Thanks @chcg for taking another look into it.

I am happy that my review could help fixing the ARM build. I think in this case, a new release of nppPluginList should be build and bundeled into Notepad++ V8.0 to get the full ARM experience.


Below the line I want to share a link to stackoverflow, which I found yesterday evening during some researches to understand the .rc script. Maybe it is interesting for others which come along, or otherwise at least for me to keep it in mind. The "magic define numbers" are responsible to embed the .json files to the .dll libs. So @chcg is indeed right that previously the x64 list was embedded into ARM.

https://stackoverflow.com/questions/2933295/embed-text-file-in-a-resource-in-a-native-windows-application