From: Maxime Devos <maximedevos@telenet.be>
To: Nala Ginrut <nalaginrut@gmail.com>
Cc: "mikael@djurfeldt.com" <mikael@djurfeldt.com>,
Adam Faiz <adam.faiz@disroot.org>,
guile-devel <guile-devel@gnu.org>,
Ricardo Wurmus <rekado@elephly.net>, Tomas Volf <~@wolfsden.cz>
Subject: RE: [PATCH] test-suite: Add tests for `for-rdelim-in-port`-related functions.
Date: Fri, 20 Dec 2024 12:51:37 +0100 [thread overview]
Message-ID: <20241220125141.qzrg2D00G2kJuzj06zrg2X@albert.telenet-ops.be> (raw)
In-Reply-To: <CAPjoZocgX4ks_w0L0CGHEWdjucEsiraLhv7TQ_X4NcE=jQ+98A@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5178 bytes --]
>>Also, you are assuming “\n” is a line delimiter. This is true under Unix according to the documentation. But it doesn’t say anything about non-Unix systems.
>RnRs defined read-line to handle different newline properly.
It’s named (ice-9 rdelim) not (rnrs rdelim). Perhaps (ice-9 rdelim) could defer to RnRS, but its documentation currently doesn’t. (If it does defer, it also needs to document what additional newline separators it recognises.)
>My original idea is to stick to a pure line string reader iterator helper function. So we can just use read-line to make things simpler. The more general implementation has to consider this issue out of the standard API.
What issue? You aren’t mentioning any bug in this paragraph, and the feature missing is also covered by the general interface (since it is general). If you are referring to the newline thing: naturally, a general API _wouldn’t_ have to consider what the behaviour of read-line is or should be, since it is up to the caller to decide what behaviour (and hence, which passed procedure) they want. (Except where used in the test cases -- to test the general interface, some specific reader needs to be passed.)
>The proposed generalisation could introduce extra complexity and seems like over engineering. But as I said, it's still beautiful way to go and I don't against such an effort. Only if we can spend extra effort to make it work properly.
What extra complexity, and what extra effort, and what doesn’t work properly about it? The generalisation I proposed:
• does not introduces any complexity – in fact, it’s less, since it does not have to have any knowledge about the ‘settings’ of (ice-9 rdelim)
• is not any more work than the specific application - in fact, it is slightly less, because of the previous point (or about equal since a single (and only a single) extra argument needs to be passed)
• in another sense, there is no extra work, since its implementation is already provided
• in another sense, there is _less_ work, since generalisations are more broadly usable (instead of having to re-implement the thing for each differerent reader(get-u8,read-json,read-line,etc.), now you could use the generalisation for all of them)
• it is a fairly minimal generalisation and this generalisation has various uses, so there is no overengineering.
I keep hearing claims about complexity, over-engineering, extra work, but nobody actually says what the complexity, over-engineering or extra work _is_.
As I’ve written the above kind of response (in less detail) multiple times, yet I keep hearing “but brr complexity/…, and no I’m not telling you what this supposed complexity/… is, and I’m ignoring the evidence(not just disagreeing on some particular points, but _ignoring_)”, by now I have to assume malice (“proof by assertion” / “… ad nauseam” is a fallacy, and not the kind to make by accident).
Like, in all the time that was spent claiming without evidence that this is overengineered, and claiming that it’s too much work/... while ignoring evidence to the contrary, the zero-additional-effort zero-additional-complexity generalisation could have been integrated in the patch and in Guile, and other things in Guile could perhaps have been improved as well.
If you keep doing this I’m going to shorten future responses to things like ‘This ignores previous evidence to the contrary, and is simply ad nauseum.’, as apparently there is some selective hearing going on so a full response is not worth the effort.
> Alright, I just elaborate my opinion before in case any misunderstanding of my previous words. Let's face the problem.
>I remember there's way to detect the current platform on the fly, so that we can test for each supported OS.
>How about Guile wrapped (uname), I used it in GNU Artanis to detect the kernel version. But there's also brief OS info.
Guile already has knows what system it is on, see e.g. %host-type. There is no need for an additional syscall at runtime.
This seems overly complicated, extra work and overengineerd, compared to simply fully documenting what (ice-9 rdelim) considers to be a line ending. Perhaps it might turn out that the current behaviour is not always desirable, but the first step is _knowing_ what the current behaviour is (the wording sounds like it is platform-dependent, but it doesn’t inspire confidence that all cases were considered, and it is impossible to determine from the documentation _what_ systems it knows about).
>I don't think we need to handle Windows case, since nowadays there's WSL and people like to run GNU things on WSL.
Untrue, see e.g. Lilypond. While WSL is at times convenient, it is not the same as native Windows support. (For example, the file names probably aren’t like C:\Dir\Subdir\…, which is incongruent with what you would expect to be able to use in a Windows application.)
Also, I was thinking on some old Mac(?) systems, where “\r” instead of “\n” is the line feed (I don’t know whether it recognised “\n”, IIRC it doesn’t).
Regards.
Maxime Devos
[-- Attachment #2: Type: text/html, Size: 11780 bytes --]
next prev parent reply other threads:[~2024-12-20 11:51 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-16 15:21 [PATCH 1/2] rdelim: Add new procedure `for-rdelim-in-port` Adam Faiz
2024-12-16 15:24 ` [PATCH 2/2] doc: Add documentation for `for-rdelim-in-port` and, related procedures Adam Faiz
2024-12-17 4:31 ` [PATCH] test-suite: Add tests for `for-rdelim-in-port`-related functions Adam Faiz
2024-12-17 5:11 ` Nala Ginrut
2024-12-17 7:21 ` Mikael Djurfeldt
2024-12-17 16:43 ` Mikael Djurfeldt
2024-12-20 9:15 ` Maxime Devos
2024-12-20 9:57 ` Nala Ginrut
2024-12-20 11:51 ` Maxime Devos [this message]
2024-12-20 12:00 ` Nala Ginrut
2024-12-20 12:53 ` Maxime Devos
2024-12-20 13:00 ` Nala Ginrut
2024-12-20 13:45 ` Maxime Devos
2024-12-20 13:52 ` Nala Ginrut
2024-12-20 14:18 ` Maxime Devos
2024-12-20 14:30 ` Nala Ginrut
2024-12-20 14:32 ` Nala Ginrut
2024-12-20 14:47 ` Maxime Devos
2024-12-20 14:56 ` Nala Ginrut
2024-12-20 15:07 ` Maxime Devos
2024-12-20 15:13 ` Nala Ginrut
2024-12-19 21:50 ` Mikael Djurfeldt
2024-12-20 15:15 ` [PATCH] test-suite: Add tests for `for-rdelim-in-port`-relatedfunctions Maxime Devos
2024-12-20 17:11 ` Mikael Djurfeldt
2024-12-20 17:31 ` Nala Ginrut
2024-12-20 18:40 ` Maxime Devos
2024-12-16 16:46 ` [PATCH 1/2] rdelim: Add new procedure `for-rdelim-in-port` Nala Ginrut
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/guile/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20241220125141.qzrg2D00G2kJuzj06zrg2X@albert.telenet-ops.be \
--to=maximedevos@telenet.be \
--cc=adam.faiz@disroot.org \
--cc=guile-devel@gnu.org \
--cc=mikael@djurfeldt.com \
--cc=nalaginrut@gmail.com \
--cc=rekado@elephly.net \
--cc=~@wolfsden.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).