Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

22 set vi name handles underscores #43

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

22 set vi name handles underscores #43

wants to merge 4 commits into from
+230 −207

Conversation

JayKayAce
Copy link

Changes made:

  • Added tests for Split Text into Words.vi
  • Added functionality to Split Text into Words.vi
  • Changed scope of the Split Text into Words.vi to community scope.
  • Added friendship with the test library for calling community VIs.

@JayKayAce JayKayAce mentioned this pull request Oct 9, 2024
Open
svelderrainruiz
svelderrainruiz requested changes Oct 10, 2024
View reviewed changes
Copy link
Collaborator

@svelderrainruiz svelderrainruiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried opening your lvproj on 2 different systems using LV21 x86 and got the same issue.

image

Make sure you fix the package dependencies warning. Did you tried running "Prepare LV to Use Icon Editor Source.vi" before making your changes? if unsure, try running it, if it gives you a popup, that means you havent run it yet on that repo before starting to make your changes.

This may not be the only issue that this PR has, but i am hopeful that fixing this would make other issues go away too.

svelderrainruiz
svelderrainruiz requested changes Oct 10, 2024
View reviewed changes
Copy link
Collaborator

@svelderrainruiz svelderrainruiz left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you removed project items that were missing. We need this to happen, but need this issue to be addressed by its own github issue, since if we somehow decide to revert the contents of this PR, this would bring back the dependency issue that you fixed.

Please revert this change, it will be addressed on the following github issue that has its own branch.
/issues/45

@JayKayAce
reverted project file to initial state for this branch, and updated w…
3e73028 
…ith the test library for Text-Based VI tests
@JayKayAce
Copy link
Author

Thanks for the feedback.
I have reverted the project file, and re-added the test library.
Looking over the changes I think the Package dependency was created by mistake when I opened the project file via JKI Dragon, but I am not entirely sure. It should at least be fixed now.
Further more when inspecting the project file I see many changes in the URIs for the listed files, could this be because I am editing the project in LV2024Q3@32Bit?

@JayKayAce
Copy link
Author

image
This missing file is now reverted again

@svelderrainruiz
Copy link
Collaborator

Thanks for the feedback. I have reverted the project file, and re-added the test library. Looking over the changes I think the Package dependency was created by mistake when I opened the project file via JKI Dragon, but I am not entirely sure. It should at least be fixed now. Further more when inspecting the project file I see many changes in the URIs for the listed files, could this be because I am editing the project in LV2024Q3@32Bit?

I am also getting many changes on the project file. Not sure what it is, but does needs to get fixed on a separate issue.
I am using LV2021 SP1 X86 to edit the source.
There are no indications on readme.md of not using the LV2024's feature of saving the source into a previous version, but it would be good to understand if LV2024 is what caused you this issue, so that the readme.md can be updated with an indication to use only LV2021 IDE

@svelderrainruiz
Copy link
Collaborator

@JayKayAce i cant seem to find the issue for this pull request. Would you mind making one?

@svelderrainruiz
Copy link
Collaborator

@JayKayAce nevermind.. i think i found it, please confirm this is the one /issues/22

@svelderrainruiz svelderrainruiz added the enhancement New feature or request label Oct 17, 2024
@svelderrainruiz svelderrainruiz linked an issue Oct 17, 2024 that may be closed by this pull request
Wanted: Handle underscores in filename when using "Set Icon to VI Name" #22
Open
@JayKayAce
Copy link
Author

that is correct this is linked to issue #22

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@svelderrainruiz svelderrainruiz svelderrainruiz requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@JayKayAce JayKayAce

Labels
enhancement New feature or request
Projects
Icon Editor
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Wanted: Handle underscores in filename when using "Set Icon to VI Name"
2 participants
@JayKayAce @svelderrainruiz