Closed pravynandas closed 3 years ago
Hi Lenilsondc,
I'm glad my PR is under review. I really loved your work on this repo and wanted to have it part of project @vbsnext. Since I did not find it on npmjs I wanted to have it published under scope @vbsnext and did it. However, please note the package is still pointing your original repository and the author in package.json is still have your name.
I was not sure, if I would be able to reach you to request considering the urgency of developing some other package depending on it, hence published one for myself.
If you have no objection, the entry @.***/vbs-pretty will continue to have you as author and always points to your git repo as primary repository. I guess you can still publish a new package "vbspretty" without @vbsnext scope.
Let me know what you think.
Kind regards, Praveen
On Sun, Jul 11, 2021 at 10:54 AM Lenilson de Castro < @.***> wrote:
Hey, thanks for your PR.
I don't see any fix in this PR, it seems that you are only tying to change the package in order to publish it into your npm account under the name @vbsnext/vbs-pretty.
I'm not quite clear on what are your goals here but if you intend to use it in other packages I can publish that into npm myself. The module is a bit outdated, as you can see, it was created a long time ago, but with a couples of tweaks I can make it available on npm so that you can use it on your projects.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/lenilsondc/vbspretty/pull/2#issuecomment-877812895, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA7I4NMMBCGTANA3I7FPY3DTXGWEBANCNFSM473YN2MA .
--
Regards, Praveen Kumar Nandagiri Quality Engineer & Technical Lead 647-893-3032
By the way the PR does include a code fix which combines continuous strings into one. Can you please review again https://github.com/lenilsondc/vbspretty/pull/2/commits/9e6009911bdc5ad273ed98bb683357776837ff41
On Sun, Jul 11, 2021 at 5:49 PM Praveen Kumar NANDAGIRI < @.***> wrote:
Hi Lenilsondc,
I'm glad my PR is under review. I really loved your work on this repo and wanted to have it part of project @vbsnext. Since I did not find it on npmjs I wanted to have it published under scope @vbsnext and did it. However, please note the package is still pointing your original repository and the author in package.json is still have your name.
I was not sure, if I would be able to reach you to request considering the urgency of developing some other package depending on it, hence published one for myself.
If you have no objection, the entry @.***/vbs-pretty will continue to have you as author and always points to your git repo as primary repository. I guess you can still publish a new package "vbspretty" without @vbsnext scope.
Let me know what you think.
Kind regards, Praveen
On Sun, Jul 11, 2021 at 10:54 AM Lenilson de Castro < @.***> wrote:
Hey, thanks for your PR.
I don't see any fix in this PR, it seems that you are only tying to change the package in order to publish it into your npm account under the name @vbsnext/vbs-pretty.
I'm not quite clear on what are your goals here but if you intend to use it in other packages I can publish that into npm myself. The module is a bit outdated, as you can see, it was created a long time ago, but with a couples of tweaks I can make it available on npm so that you can use it on your projects.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/lenilsondc/vbspretty/pull/2#issuecomment-877812895, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA7I4NMMBCGTANA3I7FPY3DTXGWEBANCNFSM473YN2MA .
--
Regards, Praveen Kumar Nandagiri Quality Engineer & Technical Lead 647-893-3032
--
Regards, Praveen Kumar Nandagiri Quality Engineer & Technical Lead 647-893-3032
I see, I'm glad this project is useful for someone else.
Right now I'm focused in fixing the bug, as I stated in the issue https://github.com/lenilsondc/vbspretty/issues/1#issuecomment-877885614 the problem is related with the escaped double quotes, and it should be addressed at the parsing phase.
I appreciate the PR but the fix is not quite suitable, it should be fixed at the parsing phase instead of a postfix like you did. The problem is instead of parsing the string bellow as two separate string, they should be a single one.
Dim myStr : myStr = "this should be parsed "" as a single string"
Currently it parses the string like so:
[
...
'"this should be parsed "', //--> STRING
'" as a single string"', //--> STRING
...
]
It should be parsed like so:
[
...
'"this should be parsed "" as a single string"', //--> STRING
...
]
I'll provide a fix ASAP. Will you be able to use it on your project if I publish this as a standalone npm package so that you can use it on your project as you wish?
Awesome, thanks!
Yup, after all your are the creator of this awesome piece and I would love to use it as a standalone dependency (without @vbsnext scope). I will unpublish the scoped one sometime today. Please keep me posted once the base package is published on npmjs. Waiting eagerly.
Glad to have found you active again.
Kind regards, Praveen
On Sun, Jul 11, 2021 at 8:28 PM Lenilson de Castro @.***> wrote:
I see, I'm glad this project is useful for someone else.
Right now I'm focused in fixing the bug, as I stated in the issue #1 (comment) https://github.com/lenilsondc/vbspretty/issues/1#issuecomment-877885614 the problem is related with the escaped double quotes, and it should be addressed at the parsing phase.
I appreciate the PR but the fix is not quite suitable, it should be fixed at the parsing phase instead of a postfix like you did. The problem is instead of parsing the string bellow as two separate string, they should be a single one.
Dim myStr : myStr = "this should be parsed "" as a single string"
Currently it parses the string like so:
[ ... '"this should be parsed "', //--> STRING '" as a single string"', //--> STRING ...]
It should be parsed like so:
[ ... '"this should be parsed "" as a single string"', //--> STRING ...]
I'll provide a fix ASAP. Will you be able to use it on your project if I publish this as a standalone npm package so that you can use it on your project as you wish?
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/lenilsondc/vbspretty/pull/2#issuecomment-877888813, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA7I4NJ3NBNQHPPDBFFASYLTXIZLZANCNFSM473YN2MA .
--
Regards, Praveen Kumar Nandagiri Quality Engineer & Technical Lead 647-893-3032
The fix applied at parser level works great. This PR need not be merged. Thanks for the fix.
Fixes https://github.com/lenilsondc/vbspretty/issues/1
Consecutive STRINGs will be joined.