From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp12.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms9.migadu.com with LMTPS id N2UIO2fVWGTZ1AAASxT56A (envelope-from ) for ; Mon, 08 May 2023 12:56:40 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp12.migadu.com with LMTPS id iHvgOWfVWGSMdgEAauVa8A (envelope-from ) for ; Mon, 08 May 2023 12:56:39 +0200 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id A876115621 for ; Mon, 8 May 2023 12:56:39 +0200 (CEST) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pvyX8-0003QH-Bs; Mon, 08 May 2023 06:55:58 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pvyX7-0003M0-0U for emacs-orgmode@gnu.org; Mon, 08 May 2023 06:55:57 -0400 Received: from mout02.posteo.de ([185.67.36.66]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pvyX4-00039h-Fg for emacs-orgmode@gnu.org; Mon, 08 May 2023 06:55:56 -0400 Received: from submission (posteo.de [185.67.36.169]) by mout02.posteo.de (Postfix) with ESMTPS id 167202402E5 for ; Mon, 8 May 2023 12:55:51 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017; t=1683543352; bh=olxryJcS4LFI9uUBXaw90RGnOFzKrPW1P140PYID+cc=; h=From:To:Cc:Subject:Date:From; b=WkIDh8B9CrqbP7W3eARH43eznvuuTi1EhFgxdEMlrAI1RFDqUHVRZoJCCikuG48a6 DkTcWCJSwoQmX+lEkyc9DR0zw2qWk+fU6Z/Ki1YhpHM5p/FYj1WKzkVrfmoApHe2sp q1AADQXkBGL34g6pHRG6zTH9DpSW2KORLxF/eDLP9WO9XSlxIP+6kKeOvSHr1Tugs6 ZYIerGCd/X+plFpVPYkxHaiu+ueCyQZ5DhK4LfGvJM4ZAKMBBGy1TkD5sedczR+Uea Tff4lUZbTrvxMYHjGQKs8UByjMj6waSuOySo0qtbpBYpDZZjiMaTulnHs8AdhU11qK dgsxNgQaoidWQ== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4QFJ9g4Xshz6tvC; Mon, 8 May 2023 12:55:51 +0200 (CEST) From: Ihor Radchenko To: Bruno Barbier Cc: emacs-orgmode@gnu.org Subject: Re: [PATCH] Add tests for ob-haskell (GHCi) In-Reply-To: <6457663a.df0a0220.f6b38.a05e@mx.google.com> References: <6416d214.5d0a0220.d9c1.54aa@mx.google.com> <875yaxvy93.fsf@localhost> <6416e4d9.df0a0220.ce03d.5c4b@mx.google.com> <87h6udozvh.fsf@localhost> <87r0texwor.fsf@localhost> <641ec686.050a0220.16dbc.a6ca@mx.google.com> <87fs9rvpyj.fsf@localhost> <6457663a.df0a0220.f6b38.a05e@mx.google.com> Date: Mon, 08 May 2023 10:59:10 +0000 Message-ID: <873547ks1t.fsf@localhost> MIME-Version: 1.0 Content-Type: text/plain Received-SPF: pass client-ip=185.67.36.66; envelope-from=yantar92@posteo.net; helo=mout02.posteo.de X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-orgmode@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+larch=yhetil.org@gnu.org Sender: emacs-orgmode-bounces+larch=yhetil.org@gnu.org X-Migadu-Country: US X-Migadu-Flow: FLOW_IN ARC-Seal: i=1; s=key1; d=yhetil.org; t=1683543399; a=rsa-sha256; cv=none; b=AqVjXPUm4XO0hAlA0oGXLeubdU+yQXyN6uuvtzy2O1RVPQJw43mke6K7/dLceWLS4jDuVB 1T92O6l8yMndsR+B0fa88WmUl1nq0fzDEYdXnaDY5F/N8fElwn4uOu5zZhGhRDVvU+hNi6 FZ9BGH2+stsSdJiCAOTah2cLBb2+u+io0MGQmtiNbZly85D9O8pndJhFXnPC/xv0rtpXor BWF02blsQL9tV+n5+Qqh7uImbdz3m+ipEvDW03rz2wz60vjEQvoB4rzNnB1gj9kRRGWmZo JT5dn0+qwDqv/+/S5neNvcD6iLtiqayWdIjCyvpLT2tT/5+zTzKy9hFFmoH9bA== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=pass header.d=posteo.net header.s=2017 header.b=WkIDh8B9; dmarc=pass (policy=none) header.from=posteo.net; spf=pass (aspmx1.migadu.com: domain of "emacs-orgmode-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="emacs-orgmode-bounces+larch=yhetil.org@gnu.org" ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1683543399; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post:dkim-signature; bh=a8uWNXmLofJV+cJxAphFQ4HxVEh2Iz6MEM48jBoXS14=; b=gt0Wb32rEuDjS8Jm16r/M8MUvn4GkIRENYtcuRVVjfAM/IhatWT7JIm1/dRj5DUgbLgBQU 6tvDT+J+LQHVDngic/U1wZKiQeEw2/pwVUQi4hON2f0ZQDNeKNUZCCJmB0ryLbeLa0hOyQ 84RpgMymZjk0o/CrMyEyOhdWrEfmOe9VoBog+R2gnb9YE6gW+z6WhJhMY9XJSYz+7V0VzL wWx0t0+I5vG3cQ346BjmWZSOFLd20i4AAj7OakARP+1S1yEu4aFolJBP63pbrpN4+Npdqo BzBMC/I8Kpq08vM/KeFvuAFH3yEmpK4n771AvZd1Pz1gPWdwKVilTtFUpWbDsw== X-Migadu-Spam-Score: -3.08 X-Spam-Score: -3.08 X-Migadu-Queue-Id: A876115621 X-Migadu-Scanner: scn0.migadu.com Authentication-Results: aspmx1.migadu.com; dkim=pass header.d=posteo.net header.s=2017 header.b=WkIDh8B9; dmarc=pass (policy=none) header.from=posteo.net; spf=pass (aspmx1.migadu.com: domain of "emacs-orgmode-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="emacs-orgmode-bounces+larch=yhetil.org@gnu.org" X-TUID: n4un7xzgJtTg Bruno Barbier writes: > Let me know if you see further improvement before pushing this. Thanks for the update! I can see that you limited the tests scope to :session blocks. Would it be possible to extend the existing tests to :compile yes case? >From a glance, it does not look like you need to change much - Haskell behaviour should be similar with or without ghci. > From 9972b926f55cb970e0b520f8726a3684118017b6 Mon Sep 17 00:00:00 2001 > From: Ihor Radchenko > Date: Fri, 24 Mar 2023 11:20:22 +0100 > Subject: [PATCH 02/13] org-babel-haskell-initiate-session: Remove secondary > prompt > > * lisp/ob-haskell.el (org-babel-haskell-initiate-session): Set > secondary prompt to "". If we do not do this, org-comint may treat > secondary prompts as a part of output. > + (sleep-for 0.25) > + ;; Disable secondary prompt. It would be useful to explain the purpose of disabling the secondary prompt in the source code comment itself, not just in the commit message. It will improve readability. > From 352d18399961fedc45cc2d64007016426e1ecd40 Mon Sep 17 00:00:00 2001 > From: Ihor Radchenko > Date: Fri, 24 Mar 2023 11:26:00 +0100 > Subject: [PATCH 04/13] * testing/lisp/test-ob-haskell-ghci.el: Enable fixed I do not see PATCH 03/13 in the attachments. > From 7d66cff5cc23bb786cb2843f4326d2869512ccac Mon Sep 17 00:00:00 2001 > From: Bruno BARBIER > Date: Sat, 25 Mar 2023 10:06:44 +0100 > Subject: [PATCH 06/13] ob-haskell: Implement sessions > > + (unless session-name > + ;; As haskell-mode is using the buffer name "*haskell*", we stay > + ;; away from it. > + (setq session-name (generate-new-buffer-name "*ob-haskell*"))) > + (let ((session (get-buffer session-name))) session is not a buffer or nil, if no buffer named session-name exists. > + (save-window-excursion > + (or (org-babel-comint-buffer-livep session) Below, (org-babel-comint-buffer-livep session) is nil, which implies either that session is nil, does not exist, not live, or does not have a process attached. > + (let ((inferior-haskell-buffer session)) > + (when (and (bufferp session) (not (org-babel-comint-buffer-livep session))) (not (org-babel-comint-buffer-livep session)) is always t here. Also, session may be a killed buffer object. It is still a buffer, but not usable. See `buffer-live-p'. > + (when (bufferp "*haskell*") (error "Conflicting buffer '*haskell*', rename it or kill it.")) > + (with-current-buffer session (rename-buffer "*haskell*"))) So, you are now renaming the unique session buffer back to "*haskell*". And never rename it back to expected :session . Users might be confused. > + (save-window-excursion > + ;; We don't use `run-haskell' to not popup the buffer. > + ;; And we protect default-directory. > + (let ((default-directory default-directory)) > + (inferior-haskell-start-process)) This is a workaround for a nasty side effect of running `inferior-haskell-start-process'. We should report this to haskell-mode developers, leaving appropriate comment in the code. > + (sleep-for 0.25) > + (setq session inferior-haskell-buffer) > + (with-current-buffer session (rename-buffer session-name)) This generally looks like a brittle workaround for inner workings of haskell-mode. I recommend sending an email to haskell-mode devs, requesting multiple session support. Otherwise, this whole code eventually be broken. > Subject: [PATCH 10/13] * testing/lisp/test-ob-haskell-ghci.el: Test output > without EOL > ... > +(ert-deftest ob-haskell/output-without-eol-1 () > + "Cannot get output from incomplete lines, when entered line by line." > + :expected-result :failed > + (should (equal "123" > + (test-ob-haskell-ghci ":results output" " > + putStr(\"1\") > + putStr(\"2\") > + putStr(\"3\") > + putStr(\"\\n\") > +")))) May you explain more about this bug? > Subject: [PATCH 11/13] lisp/ob-haskell.el: Fix how to use sessions > > + (org-babel-haskell-with-session This kind of names are usually dedicated to macro calls. But `org-babel-haskell-with-session' is instead a function. I think a macro will be better. And you will be able to get rid of unnecessary lambda. > + params > + (lambda (session) > + (cl-labels > + ((csend (txt) > + (eom () > + (with-output (todo) When using `cl-labels', please prefer longer, more descriptive function names. These functions do not have a docstring and I now am left guessing and reading the function code repeatedly to understand the usage. > + (full-body (org-babel-expand-body:generic > + body params > + (org-babel-variable-assignments:haskell params))) I think we want `org-babel-expand-src-block' here instead of using semi-internal ob-core.el parts. > - (let ((buffer (org-babel-haskell-initiate-session session))) > + (let ((buffer (org-babel-haskell-initiate-session session params))) PARAMS argument is ignored by `org-babel-haskell-initiate-session'. I am not sure why you are trying to pass it here. > Subject: [PATCH 12/13] * testing/lisp/test-ob-haskell-ghci.el: Modify > `test-ob-haskell-ghci` Here and in some other patches you are undoing changes made in previous patches. May you please consolidate transient changes by squashing commits? It will make further reviews easier. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at . Support Org development at , or support my work at