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

Allow lambdas in rg-custom-type-aliases #37

Merged
merged 2 commits into from
May 22, 2018

Conversation

muirmanders
Copy link
Contributor

Now in addition to (string . string) cons cells you can add lambda(s)
that return (string . string) cons cells. The deferred evaluation lets
you build dynamic types based on e.g. the current project, current
mode, current file path, etc.

Now in addition to (string . string) cons cells you can add lambda(s)
that return (string . string) cons cells. The deferred evaluation lets
you build dynamic types based on e.g. the current project, current
mode, current file path, etc.
@coveralls
Copy link

coveralls commented May 3, 2018

Coverage Status

Coverage increased (+0.4%) to 76.563% when pulling 901794b on muirmanders:lambda-custom-types into 4eaf35f on dajva:develop.

@coveralls
Copy link

coveralls commented May 3, 2018

Coverage Status

Coverage increased (+0.4%) to 76.563% when pulling c804d88 on muirmanders:lambda-custom-types into 4eaf35f on dajva:develop.

"Get alist of custom type aliases.
Any lambda elements will be evaluated, and nil results will be
filtered out."
(delq nil (mapcar
Copy link
Owner

Choose a reason for hiding this comment

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

Why do you filter out nil? Is this expected as a benign value to be ignored or just a way to filter out obviously wrong values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am attempting to filter out empty return values from any lambdas that returned nothing.

Copy link
Owner

Choose a reason for hiding this comment

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

Sorry if being unclear. My point was mainly if you meant to hav nil as part of the interface of lambdas as custom aliases. With this implementation, the user can return nil and the lambda will be ignored. Is that your intention?
If that's the case, it would be good if this was better documented as part of the rg-custom-type-aliases.
If this is just a way to filter out an obvious error I would prefer it being signaled to the user instead of silently dropped.

Speaking of errors, it would be good to have some sanity checks here, like applying the below function to every item of the result:

(defun rg-is-type-alias (alias)
  (and (consp alias)
       (stringp (car alias))
       (stringp (cdr alias))))

Sanity checks is not something you need to implement as part of this pr. Unless you want to, of course. :)

Copy link

Choose a reason for hiding this comment

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

I've updated the doc string to indicate nil is a valid return value should the lambda have no aliases to add.

@dajva
Copy link
Owner

dajva commented May 6, 2018

I'd like to see some documentation in the rg.el Commentary section and also in the README.md

- mention in variable doc that can be returned if the lambda has no
  aliases to add
- add lambda example to commentary and readme
@dajva dajva merged commit 31d4c66 into dajva:develop May 22, 2018
@dajva
Copy link
Owner

dajva commented May 22, 2018

Thanks a lot!

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.

4 participants