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

Refactor 'marked-form' into 'at-mark' and re-implement 'marked-form' using it. #149

Merged
merged 2 commits into from
Dec 23, 2020

Conversation

giodamelio
Copy link
Contributor

I was trying to use eval.marked-form in a binding and couldn't hard code a mark name, so this happened.

I wasn't super sure on the names, but I didn't want to break backwards compatibility on marked-form so I left it the same.

One note, I tried running make compile as described in the contributing docs, but it changes a lot of things that don't seem to be related to my change. Did I mess something up? I will include them as a separate commit so it is easier to review, and if they do look ok then I can rebase them up before this gets merged.

@Olical Olical self-assigned this Dec 22, 2020
@Olical Olical added this to the v4.11.0 milestone Dec 22, 2020
@Olical Olical added the enhancement New feature or request label Dec 22, 2020
@Olical
Copy link
Owner

Olical commented Dec 23, 2020

Thanks a lot! I'll merge and recompile myself to see what happens, I would imagine it's a platform / lua version thing? Some of the changes are just table items in a different order, I've noticed some stability issues in table sorting that may or may not be race conditions in Fennel / Lua. Not a big deal, but can sometimes produce weird diffs, it's no big deal 🙂

I'll have a think about the names, but maybe these are fine. I might shuffle them a small amount.

@Olical Olical changed the base branch from master to develop December 23, 2020 12:26
@Olical Olical merged commit 3ebe55d into Olical:develop Dec 23, 2020
@Olical
Copy link
Owner

Olical commented Dec 23, 2020

It might've been different Fennel versions, I use make (no args) when rebuilding since that'll synchronize dependencies / sponsors list etc as well as compiling the Lua. It produces the most consistent results in general.

Also just in case you submit another PR at some point, it should be made against the develop branch since that's where I do my work, I try to keep master pointing at the latest stable version for people who like to live on the edge and have the latest update as I merge it.

@Olical
Copy link
Owner

Olical commented Dec 23, 2020

So, slight tweak, I conflated your at-mark idea back into marked-form, so now there's just the original function but it takes an optional argument. If you don't provide a mark arg it'll interactively prompt for one. Slightly more magical, but I think I prefer it this way? Any thoughts? Happy with how this behaves?

@giodamelio
Copy link
Contributor Author

Nice. That does seem better. Thanks for squaring it all away!

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

Successfully merging this pull request may close these issues.

2 participants