WalonLi / edk2-vscode

The VSCode plugin of EDKII file association.
Other
31 stars 9 forks source link

`.fdf.inc` and `.dsc.inc` should be highlighted like `.fdf` and `.dsc`, respectively #17

Closed mikebeaton closed 9 months ago

mikebeaton commented 9 months ago

Hi - .fdf.inc and .dsc.inc are standards for include files in the same format as .fdf and .dsc, repsectively, in the EDK-II repo, but these files are not currently highlighted in the same way as .fdf and .dsc, respectively. I believe it would be helpful if they were.

E.g. in NetworkPkg (but also multiple other places) we have:

Screenshot 2023-07-08 at 08 08 37

 

And here is one .fdf.inc file with current .inc highlighting, and what it would ideally look like with .fdf highlighting (just achieved by a temporary rename, here):

Screenshot 2023-07-08 at 08 08 47

 

Screenshot 2023-07-08 at 08 14 55
WalonLi commented 9 months ago

@mikebeaton Thanks for your question. My perspective was mentioned in https://github.com/WalonLi/edk2-vscode/issues/8 I think this file association was edk2 implementer idea but that's not defined in spec so I don't prefer adding that.

mikebeaton commented 9 months ago

Thank you for the link to the other issue, with a reference to the right place to modify the setting. Apologies for not finding it myself, or I would have commented there.

Do you have a reference to the particular standards document which you are thinking of? (There are of several UEFI documents.) Or is it more just that these extensions aren't defined or mentioned anywhere, while .fdf and .dsc are? (Which I would accept matches my own knowledge of the documentation.)

If I might add an additional comment - though of course it's your project and your decision - it seems to me that something that's widely used in the EDK-II codebase (and this is widely used, as a quick find . -name shows) remains a reasonable candidate for inclusion in a project called edk2-vscode. I'm not saying that every mistake they may have made in there, against their own documented standards, should be supported; but this does look very like a de facto standard, which it would be convenient (for users who often will be working with the EDK-II codebase, amongst others) to support - and I cannot actually see any plausible downside (i.e. I believe that, in the context of an EDK-II-based project, it's extremely unlikely that anybody is going to use these extensions for any other reason).

So I just thought I'd have one more go at arguing for it. Thank you for your very useful project.

*I also appreciate there's a reasonably strong argument that this could be seen as 'just a hack' - i.e. .fdf.inc and .dsc.inc are just parts of a .fdf and .dsc file - but which parts? There is no formal definition. Nevertheless, in practice, just highlighting them as if they were .fdf and .dsc does seem to work, and is helpful, I believe (and there are enough samples of this 'de facto standard' in the EDK-II codebase to confirm that this basically does work). I think, for users who care about such things, it would be reasonable to say "this is supported as a kind of hack, as a convenience for you, because the content of these files is not formally defined for EDK-II code; therefore support here may be imperfect (but has been added because it largely works, and is helpful to have)".

WalonLi commented 9 months ago

https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Documentation I believe edk2 specs are not defined clearly but it's convention for related files. Though we all know that term is include meaning, but I don't think that is good practice, especially I can't control any new file association with edk2 code owner in the future. 1

But, yes. You are the 3rd person to open an issue to complain that 😂. I agree your point that this plugin is helping programmer to develop edk2 smoothly and inc is common usage in edk2 world. Adding this support can reduce my effort to copy&paste github link to explain why I did. Ha, let me add in next release.

WalonLi commented 9 months ago

PR submitted. https://github.com/WalonLi/edk2-vscode/pull/19