search

christianhelle / reswcodegen

A Visual Studio Custom Tool for generating a strongly typed helper class for accessing localized resources from a .ResW file.
http://bit.ly/reswcodegen
GNU General Public License v3.0
11 stars 11 forks source link

Remove reference to Application.Current from generated code #157

Closed groberts-nr closed 6 months ago

groberts-nr commented 6 months ago

Calling Windows.UI.Xaml.Application.Current throws an exception if it is run in a UWP app that is not based on an Application class, for example, an immersive HoloLens app. Is there a reason to refer to the resource by its short name rather than fully qualified? If we always just use the fully qualified resource path, then we don't need to call Application.Current. Let me know what you think! Thanks!

christianhelle commented 6 months ago

@groberts-nr thanks for taking the time to implement this.

Unfortunately, I haven't done any UWP in a long time so I can't answer that. This tool dates back to Windows Phone 7 and when Windows Apps were still called Metro-style apps.

christianhelle commented 6 months ago

@groberts-nr I'll pull this branch once I get access to a Windows PC and try it out before we merge it in

christianhelle commented 6 months ago

@groberts-nr I pulled your branch but I couldn't get it to work.

I get this error image

The project is just a File -> New Project -> Blank UWP App image

The .Resw file has a single entry image

christianhelle commented 6 months ago

@groberts-nr After playing a bit more, I figured out that your branch only works when the localization resources are in a separate project from the app.

Your branch works fine with this setup image

But not with this: image

christianhelle commented 6 months ago

@groberts-nr I appreciate the effort and time you took to improve on the generated code but since this tool has other users, it needs to work for several different scenarios

You're welcome to improve further on the generated code, but as it is right now I don't think we should merge it in

groberts-nr commented 6 months ago

That makes sense, thank you for taking the time to check it out! I'll let you know if I find another solution that will work for all scenarios.

groberts-nr commented 6 months ago

Hi @christianhelle, I updated my approach to use System.Reflection.Assembly.GetEntryAssembly(). This fixes the same-project case. I'm not aware of any problematic implications but let me know what you think.

groberts-nr commented 6 months ago

@christianhelle Have you had the chance to look at the latest commit?

christianhelle commented 6 months ago

@christianhelle Have you had the chance to look at the latest commit?

@groberts-nr Sorry, things got hectic on other projects and I completely forgot about this.

I'm having a look at it now

christianhelle commented 6 months ago

Thanks for the contribution @groberts-nr

groberts-nr commented 6 months ago

Thanks so much @christianhelle ! Although now I'm realizing that this code snippet will probably cause generated Visual Basic code to fail. I'd like to have the global:: namespace in there to avoid conflicts with any user-defined System class, but I'm not sure how to do that in a language-agnostic way.

groberts-nr commented 6 months ago

@christianhelle Although it appears VB was already broken with the other code snippet Windows.UI.Core.CoreWindow.GetForCurrentThread() != null so perhaps this is not a big deal after all. Thanks again!

christianhelle commented 6 months ago

@groberts-nr I honestly don't know of anyone who still works with VB.NET, but I'll try to fix it at some point