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.

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

Add ;; #780

Closed
wants to merge 6 commits into from
Closed

Add ;; #780

wants to merge 6 commits into from

Conversation

tschettervictor
Copy link
Collaborator

No description provided.

@tschettervictor
Copy link
Collaborator Author

@yaazkal Can you test this one? I somehow forgot a ;; at the end of the case command.

@bmac2
Copy link
Collaborator

bmac2 commented Dec 31, 2024

I assume this is supposed to be exactly like #770 with the one line fix, but this one is missing other lines that were in 770. I am confused. Is the else line needed or not?

@tschettervictor
Copy link
Collaborator Author

Yes.

That line is at the end of every case command. I'm not sure why the checks aren't catching it.

@tschettervictor
Copy link
Collaborator Author

Out maybe it dude need to be present in a catch all case command.

Are you seeing the same error as @yaazkal ?

@bmac2
Copy link
Collaborator

bmac2 commented Dec 31, 2024

no I am just comparing the two PRs and there are other lines in 770. Specifically a line with:
else
on it missing in this one. Was that otherline an error too?

@bmac2
Copy link
Collaborator

bmac2 commented Dec 31, 2024

the ;; I get and understand

@tschettervictor
Copy link
Collaborator Author

Else is missing? Can you point it out?

@bmac2
Copy link
Collaborator

bmac2 commented Dec 31, 2024

this one now tests fine for me. Tested template in normal place, then a template sitting in /tmp for the other test. Worked through it with @tschettervictor so this one needs another round of testing @yaazkal

@yaazkal
Copy link
Collaborator

yaazkal commented Jan 4, 2025

I have tested this and dosen't break anything. Now, I still agree to what @tschettervictor said in the previous PR:

After careful consideration, it's probably best to leave it as is.

Templates should follow the structure of project/template anyway in my opinion.

If you still want to continue with this PR, it will need documentation so the user dosen't get unexpected behavior or you can consider adding a flag to explicitly tell the system to use the current folder to look for templates. I guess this will be confusing.

@tschettervictor
Copy link
Collaborator Author

I'll close it. Let's leave it as is for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants