Vazor icon indicating copy to clipboard operation
Vazor copied to clipboard
verified publisher icon VBAndCs

Changes and minor refactoring to make deployments work on Linux.

Open mmkathurima opened this issue 11 months ago • 17 comments

Included:

  • Fixed inconsistent file separators across file systems as Linux combines path separators with its separator / to the incorrect Views\Shared/_Layout.cshtml when it should be Views\Shared\_Layout.cshtml. This incorrect path leads to the layout file not being found.
  • Addition of types during variable declaration.
  • Proper encapsulation.

mmkathurima avatar Feb 01 '25 20:02 mmkathurima

Looks good!

InteXX avatar Feb 01 '25 21:02 InteXX

Thanks @mmkathurima for your efforts. I can't approve this PR because O don't have full access to GitHub since they enabled 2FA. Also, you shouldn't modify any code style, focusing only the exact issues you want to fix. I already modified the tow fixes you suggested, but the second one is not published yet. I need you to confirm that this change works on Linux: Public Shared Function FixPath(path As String) As String Return path.Replace(""c, "/"c).Trim("/"c) End Function

So, I can update the Vazor Nuget and VazorEx. Please make this change at your end, and tell me the result. Thanks.

From: mmkathurima @.> Sent: Saturday, February 1, 2025 8:23 PM To: VBAndCs/Vazor @.> Cc: Subscribed @.***> Subject: [VBAndCs/Vazor] Changes and minor refactoring to make deployments work on Linux. (PR #24)

Included:

  • Fixed inconsistent file separators across file systems as Linux combines path separators with its separator / to the incorrect Views\Shared/_Layout.cshtml when it should be Views\Shared_Layout.cshtml. This incorrect path leads to the layout file not being found.
  • Addition of types during variable declaration.
  • Proper encapsulation.

You can view, comment on, or merge this pull request online at:

https://github.com/VBAndCs/Vazor/pull/24

Commit Summary

File Changes

(6 fileshttps://github.com/VBAndCs/Vazor/pull/24/files)

Patch Links:

  • https://github.com/VBAndCs/Vazor/pull/24.patch
  • https://github.com/VBAndCs/Vazor/pull/24.diff

— Reply to this email directly, view it on GitHubhttps://github.com/VBAndCs/Vazor/pull/24, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ALQ5MVR66VSMNWTW6FMFMU32NUUMNAVCNFSM6AAAAABWJT2MXKVHI2DSMVQWIX3LMV43ASLTON2WKOZSHAZDKNBRGAYDCNA. You are receiving this because you are subscribed to this thread.Message ID: @.***>

VBAndCs avatar Feb 03 '25 18:02 VBAndCs

Note that I have not modified that function. I have modified where it was called, I have referenced that change in my issue #22.

This is how it looked like and should remain as is:

Public Shared Function FixPath(path As String) As String
    Return path.Replace("/"c, "\"c).Trim("\"c)
End Function

mmkathurima avatar Feb 03 '25 19:02 mmkathurima

I will change it anyway, so please test the change upfront to be safe. The "/" is the path separator for Unix, Linux and Windows.

From: mmkathurima @.> Sent: Monday, February 3, 2025 7:03 PM To: VBAndCs/Vazor @.> Cc: Mohammad Hamdy Ghanem @.>; Comment @.> Subject: Re: [VBAndCs/Vazor] Changes and minor refactoring to make deployments work on Linux. (PR #24)

Note that I have not modified that function. I have modified where it was called, I have referenced that change it in my issue.

This is how it looked like and should remain as is:

Public Shared Function FixPath(path As String) As String Return path.Replace("/"c, ""c).Trim(""c) End Function

— Reply to this email directly, view it on GitHubhttps://github.com/VBAndCs/Vazor/pull/24#issuecomment-2631828138, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ALQ5MVVL6LN4PGMWR22EVW32N64PLAVCNFSM6AAAAABWJT2MXKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMZRHAZDQMJTHA. You are receiving this because you commented.Message ID: @.***>

VBAndCs avatar Feb 03 '25 19:02 VBAndCs

@VBAndCs

I can't approve this PR because O don't have full access to GitHub since they enabled 2FA

The fix for that is incredibly easy.

InteXX avatar Feb 03 '25 23:02 InteXX

I have tested it on both Windows and Linux and it works with /.

mmkathurima avatar Feb 04 '25 12:02 mmkathurima

@mmkathurima Thanks. I will provide updated packages soon.

From: mmkathurima @.> Sent: Tuesday, February 4, 2025 12:16 PM To: VBAndCs/Vazor @.> Cc: Mohammad Hamdy Ghanem @.>; Mention @.> Subject: Re: [VBAndCs/Vazor] Changes and minor refactoring to make deployments work on Linux. (PR #24)

I have tested it on both Windows and Linux and it works with /.

— Reply to this email directly, view it on GitHubhttps://github.com/VBAndCs/Vazor/pull/24#issuecomment-2633731235, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ALQ5MVUJWYCORA3SFTKBGAL2OCVQLAVCNFSM6AAAAABWJT2MXKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMZTG4ZTCMRTGU. You are receiving this because you were mentioned.

[https://s-install.avcdn.net/ipm/preview/icons/icon-envelope-tick-round-orange-animated-no-repeat-v1.gif]https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail Virus-free.www.avast.comhttps://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail

VBAndCs avatar Feb 04 '25 22:02 VBAndCs

Hello @VBAndCs. Were you able to update packages?

mmkathurima avatar Feb 11 '25 15:02 mmkathurima

@VBAndCs

I'm ready to deploy a working Vazor app to production in a GCR container, which of course is Linux. My app is DOA. We very much need this update.

I can't approve this PR because O [sic] don't have full access to GitHub since they enabled 2FA

The fix for that is incredibly easy.

Would you implement the (easy) 2FA fix that I've suggested, sign into your GitHub account, and process this PR.

Thank you.

InteXX avatar May 30 '25 19:05 InteXX

Hi Jeff: I am not willing to continue this project right now (may be with the next VS which I expect not to work on Win 10). You can fork Vazor and take it forward as you need. I already published the whole project (including the installer and auto-completion provider), so you have all what you need to move on. Thanks.

From: Jeff Bowman @.> Sent: Friday, May 30, 2025 7:33 PM To: VBAndCs/Vazor @.> Cc: Mohammad Hamdy Ghanem @.>; Mention @.> Subject: Re: [VBAndCs/Vazor] Changes and minor refactoring to make deployments work on Linux. (PR #24)

[https://avatars.githubusercontent.com/u/10966749?s=20&v=4]InteXX left a comment (VBAndCs/Vazor#24)https://github.com/VBAndCs/Vazor/pull/24#issuecomment-2923312089

@VBAndCshttps://github.com/VBAndCs

I'm ready to deploy a working Vazor app to production in a GCR container, which of course is Linux. My app is DOA. We very much need this update.

I can't approve this PR because I [sic] don't have full access to GitHub since they enabled 2FA

The fix for that is incredibly easy.

Would you implement the (easy) 2FA fix that I've suggested, sign into your GitHub account, and process this PR.

Thank you.

— Reply to this email directly, view it on GitHubhttps://github.com/VBAndCs/Vazor/pull/24#issuecomment-2923312089, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ALQ5MVX2YC5MI6X773RY24L3BCXAXAVCNFSM6AAAAABWJT2MXKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSMRTGMYTEMBYHE. You are receiving this because you were mentioned.

[https://s-install.avcdn.net/ipm/preview/icons/icon-envelope-tick-round-orange-animated-no-repeat-v1.gif]https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail Virus-free.www.avast.comhttps://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail

VBAndCs avatar May 31 '25 04:05 VBAndCs

@VBAndCs

OK, sounds good—will do.

Thank you for all of your hard work and innovation on this project. It truly is ground-breaking.

One quick question... you say you've published the installer source, but I'm not finding it.

Do you have a link? I'm getting a 404 from the .ZIP download link on this page, but I'm not sure if that's what you're referring to.

InteXX avatar May 31 '25 04:05 InteXX

@mmkathurima

I'm not seeing in this PR where you're addressing the path separator issue.

Could you clarify?

InteXX avatar May 31 '25 05:05 InteXX

VazorEx (which is the vixs project which contains the auto-completion provider) is a part of the Vazor repo: https://github.com/VBAndCs/Vazor/tree/master/VazorEx

From: Jeff Bowman @.> Sent: Saturday, May 31, 2025 4:44 AM To: VBAndCs/Vazor @.> Cc: Mohammad Hamdy Ghanem @.>; Mention @.> Subject: Re: [VBAndCs/Vazor] Changes and minor refactoring to make deployments work on Linux. (PR #24)

[https://avatars.githubusercontent.com/u/10966749?s=20&v=4]InteXX left a comment (VBAndCs/Vazor#24)https://github.com/VBAndCs/Vazor/pull/24#issuecomment-2924259259

@VBAndCshttps://github.com/VBAndCs

OK, sounds good—will do.

Thank you for all of your hard work and innovation on this project. It truly is ground-breaking.

One quick question... you say you've published the installer source, but I'm not finding it.

Do you have a link? I'm getting a 404 from the .ZIP download link on this pagehttps://github.com/VBAndCs/Xml-Literals-Completion-Provider, but I'm not sure if that's what you're referring to.

— Reply to this email directly, view it on GitHubhttps://github.com/VBAndCs/Vazor/pull/24#issuecomment-2924259259, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ALQ5MVU2NE2MQJTNBVDLJJ33BEXUNAVCNFSM6AAAAABWJT2MXKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSMRUGI2TSMRVHE. You are receiving this because you were mentioned.Message ID: @.***>

VBAndCs avatar May 31 '25 05:05 VBAndCs

@VBAndCs

VazorEx

Got it, thanks.

Fix your GitHub account! It'll take you five minutes.

InteXX avatar May 31 '25 05:05 InteXX

@mmkathurima

I'm not seeing in this PR where you're addressing the path separator issue.

Could you clarify?

The PR is in the ZML project.

P.S. Once you fork the project I suggest using strict variable/return type declarations eg Dim i = 10 becomes Dim i As Integer = 10 or Public Function Factorial(i As Integer) As Integer instead of Pubic Function Factorial(i) as I have done in my PRs

mmkathurima avatar May 31 '25 07:05 mmkathurima

@mmkathurima

strict variable/return type declarations

Yes, I agree. In fact, I always take it one step further.

Excepting anonymous types of course, I never declare variables inline. I always declare them at the top of the method body and then initialize them later.

But I won't be forking the Vazor repo, at least not now. Like Mohammad, my own time constraints are considerable currently. I must settle for keeping my improvements internal for the time being. I'm afraid I just don't have the capacity right now to respond to PRs and Issues.

Hopefully I'll be able to do all of this publicly soon.

InteXX avatar May 31 '25 18:05 InteXX

@mmkathurima

I won't be forking the Vazor app

But I can say that the solution to the path separator issue lies in altering the ViewInfo.FixPath() function to use OS-sensitive framework calls:

Public Shared Function FixPath(ViewPath As String) As String
  Return ViewPath.Replace("/"c, Path.DirectorySeparatorChar).Trim("\"c, "/"c)
End Function

Also, don't hardcode your view paths. I ended up doing this:

Namespace Views.Home
  Public Class IndexView
    Inherits VazorView

    Private Sub New(ViewData As ViewDataDictionary)
      MyBase.New("Index", View.GetPath(NameOf(Home)), NameOf(Home))

      ViewData(NameOf(Me.Title)) = Me.Title
    End Sub
  End Class
End Namespace

Namespace Views
  Friend Class View
    Friend Shared Function GetPath(ViewName As String) As String
      Return Path.Combine("Views", ViewName)
    End Function
  End Class
End Namespace

InteXX avatar Jun 01 '25 00:06 InteXX