magnars / dash.el

A modern list library for Emacs
GNU General Public License v3.0
1.66k stars 138 forks source link

[QUESTION] Does `-some-->` return non-nil when forms contain nil directly? #341

Closed HKey closed 3 years ago

HKey commented 4 years ago

Hi.

When calling -some--> like (-some--> 1 nil), it returns 1. I expected that the result would be nil. Of course it is bad usage because it doesn't use it, but is it the expected result for -some--> that returns non-nil when forms contain nil directly?

basil-conto commented 4 years ago

This seems to be an artefact of the macro's argument list: (x &optional form &rest more).

In particular, (-some--> 1 nil) is the same as (-some--> 1) due to how Elisp handles &optional arguments.

I'd have to check more closely, but off the top of my head this may be fixable by changing the arglist to (x &rest more).

magnars commented 4 years ago

This isn't something worth fixing, since (-some--> 1 nil) doesn't make any sense. It would expand to the nonsensical (nil 1)

On Tue, Aug 4, 2020 at 9:23 AM Basil L. Contovounesios < notifications@github.com> wrote:

This seems to be an artefact of the macro's argument list: (x &optional form &rest more).

In particular, (-some--> 1 nil) is the same as (-some--> 1) due to how Elisp handles &optional arguments.

I'd have to check more closely, but off the top of my head this may be fixable by changing the arglist to (x &rest more).

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/magnars/dash.el/issues/341#issuecomment-668428133, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACA4ON4V2HFSKYK5SOFILLR66ZOPANCNFSM4PT3ORQQ .

basil-conto commented 4 years ago

Sure, but it's probably worth making Dash more consistent and less surprising if reasonably possible.

magnars commented 4 years ago

Agreed. In this case, the consistent behavior would be an expansion to (nil 1) and the subsequent error (void-function nil).

HKey commented 4 years ago

In this case, the consistent behavior would be an expansion to (nil 1) and the subsequent error (void-function nil).

You are right. I missed that symbols are converted to function calling.

Changing -some--> like below then it will be fixed:

-(defmacro -some--> (x &optional form &rest more)
+(defmacro -some--> (x &rest forms)
   "When expr is non-nil, thread it through the first form (via `-->'),
 and when that result is non-nil, through the next form, etc."
   (declare (debug ->)
            (indent 1))
-  (if (null form) x
+  (if (null forms) x
     (let ((result (make-symbol "result")))
       `(-some--> (-when-let (,result ,x)
-                   (--> ,result ,form))
-                 ,@more))))
+                   (--> ,result ,(car forms)))
+                 ,@(cdr forms)))))

BTW, if I create a PR for this, FSF paperwork sign is needed?

basil-conto commented 4 years ago

BTW, if I create a PR for this, FSF paperwork sign is needed?

If this is the only change you've contributed to Dash, then no. But I'd encourage you to complete the paperwork if you think you might like to contribute again in the future. It's when your total contributions exceed ~15 lines of code that you need a copyright assignment. Here's the relevant form: https://git.savannah.gnu.org/cgit/gnulib.git/tree/doc/Copyright/request-assign.future

Thanks.

HKey commented 4 years ago

@basil-conto Thank you for letting me know that.

I'll do the paperwork. But this is my first time doing that so I think it may take some days.

basil-conto commented 4 years ago

That's fine, there's no rush on our side. :) Just let us know when the process is completed so we can verify that you're on file.

HKey commented 4 years ago

I've read https://www.gnu.org/prep/maintain/html_node/Copyright-Papers.html, I've understood why request-assign.future is used but I have two questions:

  1. What should I answer to questions below?

[What is the name of the program or package you're contributing to?]

dash, dash.el or Emacs?

[Which files have you changed so far, and which new files have you written so far?]

Just dash.el?

  1. In this case, if I change another file not dash.el in the future, will request-assign.future be needed for the change?
basil-conto commented 4 years ago

[What is the name of the program or package you're contributing to?]

Back when I did my CA, I said explicitly which GNU ELPA packages I had already contributed to, and that I intended to contribute to GNU Emacs itself as well, in addition to those packages. You can probably get away with just "GNU Emacs and its extensions in GNU ELPA" or something along those lines. Either way, the CA clerk will let you know if it's not right.

[Which files have you changed so far, and which new files have you written so far?]

I think you can just say that you haven't changed any files yet, but you intend to change dash.el as a first step.

In this case, if I change another file not dash.el in the future, will request-assign.future be needed for the change?

No, the CA is a once-off process for all GNU Emacs-related changes. The request-assign.future form is just asking what you've already done at the time of the application, I think. Again, if there's anything amiss, the clerk will let you know.

The form is just your way of introducing yourself to the FSF; the actual CA is a followup contract, and you'll see in its wording that there's no mention of which specific files you have changed or will change, etc.

HKey commented 4 years ago

Thank you for answering :)

I misunderstood that it would be needed each time when I change other files.

basil-conto commented 4 years ago

Thank you for your interest. :)

basil-conto commented 3 years ago

Any updates on this?

HKey commented 3 years ago

I'm sorry for delay.

I've been asking CA clerk about details of copyright assignment. I had thought copyright assignment would only cover changes that were submitted, such as pull requests, but CA clerk told me, as not legal advice, changes that aren't submitted may also be covered. The range of copyright assignment seems to be wider than I thought, so I sent an email to cancel the request-assign.future request. Currently I've not received a reply yet.

Would you treat this change as not legally significat? The next time I contribute to GNU Emacs, I'll consider choosing request-assign.changes or request-disclaim.changes.

basil-conto commented 3 years ago

I had thought copyright assignment would only cover changes that were submitted, such as pull requests, but CA clerk told me, as not legal advice, changes that aren't submitted may also be covered.

IANAL either, but the title of the request-assign.future form is REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES.

This means that if you send more pull requests to Dash, Emacs, or similar projects in the future, you won't have to re-assign your copyright; any changes that end up being merged will automatically be covered by the original request-assign.future form.

That's why I said "I'd encourage you to complete the paperwork if you think you might like to contribute again in the future," as it saves everyone time.

Would you treat this change as not legally significat?

As I said in https://github.com/magnars/dash.el/issues/341#issuecomment-669025855, changes of less than ~15 "unique" lines can be accepted without copyright assignment.

The next time I contribute to GNU Emacs, I'll consider choosing request-assign.changes or request-disclaim.changes.

Sure, whatever works for you. See (info "(maintain) Copyright Papers") for more information on these forms.

basil-conto commented 3 years ago

BTW, you can create a pull request of any size without assigning copyright; it's just that we can't use legally significant portions of your code without a copyright assignment.

HKey commented 3 years ago

IANAL either, but the title of the request-assign.future form is REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES.

This means that if you send more pull requests to Dash, Emacs, or similar projects in the future, you won't have to re-assign your copyright; any changes that end up being merged will automatically be covered by the original request-assign.future form.

That's why I said "I'd encourage you to complete the paperwork if you think you might like to contribute again in the future," as it saves everyone time.

My previous comment was ambiguous. I agree that changes end up being merged into Dash, GNU Emacs or other Emacs related projects copyrighted by FSF will be automatically covered. I was worried about that whether changes that do not end up being merged into such projects will also be covered. For example, changes for other projects not copyrighted by FSF or for personal projects.

In the pdf I received, I felt ambiguous whether changes covered by copyright assignment are limited to the changes merged into the projects copyrighted by FSF, so I asked CA clerk. The pdf I received says that the assignment applies to all past and future works of Developer that constitute changes and enhancements to the Program. "Constitute" seemed to point to changes that end up being merged into the program. On the other hand, for example, it seemed that an external package loaded at runtime also constitutes changes and enhancements to the program at runtime for me. And then I received a reply that, as not legal advice, if I write code (works) for Emacs and because those files cannot be used anywhere else but Emacs, they would constitute changes and enhancements to Emacs.

However, I am not familiar with the law and copyright assignment, and I am not a native English speaker, so this email exchange was difficult for me and I may have misunderstood something in the email exchange. Perhaps, does request-assign.future mean that regardless of whether it end up being merged or not, if I write code for Emacs, all the copyright of them will be assigned to FSF?

As I said in #341 (comment), changes of less than ~15 "unique" lines can be accepted without copyright assignment.

Thanks, I'll create a PR.

BTW, you can create a pull request of any size without assigning copyright; it's just that we can't use legally significant portions of your code without a copyright assignment.

I hesitated to create it because I remembered that the commit message would be different depending on whether I signed request-assign.future. But I could change the commit message after it was created, so I needed not to hesitate.

basil-conto commented 3 years ago

I was worried about that whether changes that do not end up being merged into such projects will also be covered. For example, changes for other projects not copyrighted by FSF or for personal projects.

No, of course not.

Perhaps, does request-assign.future mean that regardless of whether it end up being merged or not, if I write code for Emacs, all the copyright of them will be assigned to FSF?

Again, without this being legal advice: the copyright assignment covers code or documentation that both (a) you have written; and (b) is included in some FSF-copyrighted package such as Dash or GNU Emacs. It does not cover your personal projects that are not included in Dash, or are not relevant to Dash. The only reason you need to do the copyright assignment, is so that the FSF can enforce its software licenses in court against anyone who violates them, e.g. if they include GPL-ed software in a proprietary system. The FSF does not want to own everything you do. :)

Thanks, I'll create a PR.

Thanks, I look forward to it.

I hesitated to create it because I remembered that the commit message would be different depending on whether I signed request-assign.future. But I could change the commit message after it was created, so I needed not to hesitate.

Exactly.