wyuenho / emacs-pet

Tracks down the correct Python tooling executables from your virtualenvs so you can glue the binaries to Emacs and delete code in init.el
GNU General Public License v3.0
105 stars 13 forks source link

Fix incorrect pcase pattern. #35

Closed tusharhero closed 3 months ago

tusharhero commented 6 months ago

Fix for #34

wyuenho commented 6 months ago

Do you mind adding a test case for this bug please?

tusharhero commented 6 months ago

@yantar92 Can you please clarify if this is the right one? Because in you intially asked me to the diff as in this PR, but in the Github issue you did something else.

What we are currently doing:

-    (pcase-let ((`(,_ ,action ,file ,@_) event))
+    (pcase-let ((`(,_ ,action ,file) event))

What you suggested in your comment:

-    (pcase-let ((`(,_ ,action ,file ,@_) event))
+    (pcase-let ((`(,_ ,action ,file,_) event))

Both of them seem to work in my configuration.

yantar92 commented 6 months ago

tusharhero @.***> writes:

@yantar92 Can you please clarify if this is the right one? Because in you intially asked me to the diff as in this PR, but in the Github issue. You did this.

What we are currently doing:

-    (pcase-let ((`(,_ ,action ,file ,@_) event))
+    (pcase-let ((`(,_ ,action ,file) event))

This will work, because of undocumented (yet! see https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68509#20) behaviour of destructuring pcase-let.

What you suggested in your comment:

-    (pcase-let ((`(,_ ,action ,file ,@_) event))
+    (pcase-let ((`(,_ ,action ,file,_) event))

This variant will also work. It provides a correct pattern in a pcase' sense, without making use of specific behavior ofpcase-let'.

I'd personally use the second variant, but you or @wyuenho can use either.

-- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at https://orgmode.org/. Support Org development at https://liberapay.com/org-mode, or support my work at https://liberapay.com/yantar92

tusharhero commented 5 months ago

Hey @wyuenho , I fixed it to be the one suggested by yantar. I have no idea how to make a test case, can you please provide any pointers?