Add partial combined SDL2 and ncurses build by frejanordsiek · Pull Request #1329 · lem-project/lem

frejanordsiek · 2024-03-05T00:10:43Z

This adds a partial combined SDL2 and ncurses build to address #1327 . The changes add a -i,--interface INTERFACE option to specify the desired interface: sdl2 or ncurses (case-insensitive). If whatever is given is not available, the logic goes down the existing chain of finding a default implementation.

But, there are a few issues with this merge request, which are:

It doesn't auto-detect whether lem is run from a tty or not and (default on a tty should probably be ncurses, but elsewhere it might make sense for it to be SDL2). This is part of why this is a "partial" solution.
It is a bit of a hack. Again, "partial" solution
The choice of command line argument might not be the best
My Common Lisp is really rusty (haven't used it in about 6-7 years) and the indentation and choice of functions might not be the best.

vindarel · 2024-03-06T17:08:04Z

src/lem.lisp

                                         filename)))
+                                ((member arg '("-i" "--interface") :test #'equal)
+                                 (let ((interface (pop args)))
+                                   (unless (and interface (/=  0 (array-total-size interface)))


Are you looking for plusp and length for strings?

Those would work. I chose array-total-size since it only handles arrays (which strings are), but it does hurt readability and the performance loss from the type check when checking arguments is so close to zero that it is basically irrelevant. As for plusp, that would also work. Mainly make it more clear.

I've changed to them in the latest commits.

vindarel · 2024-03-06T17:09:22Z

src/lem.lisp

+                                ((member arg '("-i" "--interface") :test #'equal)
+                                 (let ((interface (pop args)))
+                                   (unless (and interface (/=  0 (array-total-size interface)))
+                                     (error "Please, specify an interface to use."))


I don't think erroring here is cool. I'd rather have a default.

Also an error throws a backtrace right? A simple message to the user might have been nicer.

Makes sense. I've changed it to printing a string to stderr in a new commit.

vindarel · 2024-03-06T17:10:28Z

src/lem.lisp

+                                   (unless (and interface (/=  0 (array-total-size interface)))
+                                     (error "Please, specify an interface to use."))
+                                   (setf (command-line-arguments-interface parsed-args)
+                                         (intern (string-upcase interface) "KEYWORD"))))


Also alexandria:make-keyword (with the string upcase)

Interns the string designated by NAME in the KEYWORD package.

but if this works it works.

I switched it to alexandria:make-keyword in a commit I just pushed to make it more clear what is happening to someone skimming the code fast.

…nt a message to stderr if an interface argument isn't given or it has zero length rather than erroring.

…ocessing in src/lem.lisp to using alexandria:make-keyword.

frejanordsiek · 2024-03-06T19:51:03Z

Bringing the discussion of the command line argument in #1327 over here.

Right now, the -i INTERFACE or --interface INTERFACE argument scheme is a bit unwieldy. The one advantage it has is that it can handle any number of interfaces, where as switches can only handle 2 very easily

Emacs uses -nw (short for --no-window-system) as its one switch.

Of course, it would be nice to make it autodetect if it is on a tty and default to using ncurses then and default to SDL2 if it isn't, but I am not sure how to do that. ncurses doesn't seem to have any function to detect this. Nor does SDL2. My best guess is to try (interactive-stream-p *standard-input*).

…n is interactive (probably a terminal) and SDL2 otherwise.

frejanordsiek · 2024-03-06T20:01:56Z

OK, I had a "duh" moment and figured out why my attempts at using (interactive-stream-p *standard-input*) were failing so badly. I just made a new commit that defaults the interface to ncurses if stdin is interactive (probably a terminal) and sdl2 if it isn't (probably launched from a Desktop app launcher or file browser).

frejanordsiek · 2024-03-15T09:24:15Z

A potentially simpler scheme for the command line arguments could be

-nw / --no-window-system from Emacs for any terminal interface (ncurses is the only one right now)
-gw / --graphical-window for any graphical interface (sdl2 is the only one right now)
-i INTERFACE / --interface INTERFACE for explicitly picking an interface by name (might be important if there is every more than one terminal and/or graphics interface).

How does this sound?

vindarel · 2024-03-15T10:58:40Z

mmh I find "-nw" not so explicit. What about

--nogui defaults to ncurses
--gui -> SDL2
and -i

but wait, simply:

--ncurses
--sdl or --sdl2
-i

frejanordsiek · 2024-03-15T11:48:04Z

mmh I find "-nw" not so explicit. What about
* `--nogui` defaults to ncurses

* `--gui` -> SDL2

* and -i
but wait, simply:
* `--ncurses`

* `--sdl` or `--sdl2`

* -i

Why not both?

--ncurses for using ncurses
--sdl and --sdl2 for using sdl2
--nogui for using whatever terminal interface is available (in case anything beyond ncurses is added)
--gui for using whatever GUI interface is available (in case anything beyond sdl2 is added)
-i / --interface for specifying interface by name with a single option.

…--ncurses, --sdl2, and --sdl.

frejanordsiek · 2024-03-16T23:50:02Z

I added them.

cxxxr · 2024-03-19T11:48:05Z

Thank you for the great PR.
I think it is generally good, but I would like to avoid unnecessarily adding more command line options.
I think "-i" "--interface" is sufficient, do we need to add other options?

frejanordsiek added 2 commits March 5, 2024 00:25

Added Makefile rule and script for building lem with SDL2 and ncurses.

36e4b7c

Added -i,--interface argument to select sdl2 or ncurses implementations.

076c71c

vindarel reviewed Mar 6, 2024

View reviewed changes

frejanordsiek added 3 commits March 6, 2024 20:23

Simplified the checking of the --interface argument in src/lem.lisp.

f535845

Changed the -i/--interface argument processing in src/lem.lisp to pri…

94e5227

…nt a message to stderr if an interface argument isn't given or it has zero length rather than erroring.

Changed the generation of the keyword for the --interface argument pr…

97896e8

…ocessing in src/lem.lisp to using alexandria:make-keyword.

Changed src/lem.lisp to default to the ncurses implementation if stdi…

0ee6b06

…n is interactive (probably a terminal) and SDL2 otherwise.

Added additional arguments to control the interface: --gui, --nogui, …

3966e8c

…--ncurses, --sdl2, and --sdl.

cxxxr merged commit 9470a90 into lem-project:main Mar 23, 2024

frejanordsiek mentioned this pull request Mar 23, 2024

Building Lem with both SDL2 and ncurses support simultaneously #1327

Closed

Uh oh!

Conversation

frejanordsiek commented Mar 5, 2024

Uh oh!

vindarel Mar 6, 2024

Choose a reason for hiding this comment

Uh oh!

frejanordsiek Mar 6, 2024

Choose a reason for hiding this comment

Uh oh!

vindarel Mar 6, 2024

Choose a reason for hiding this comment

Uh oh!

frejanordsiek Mar 6, 2024

Choose a reason for hiding this comment

Uh oh!

vindarel Mar 6, 2024

Choose a reason for hiding this comment

Uh oh!

frejanordsiek Mar 6, 2024

Choose a reason for hiding this comment

Uh oh!

frejanordsiek commented Mar 6, 2024

Uh oh!

frejanordsiek commented Mar 6, 2024

Uh oh!

frejanordsiek commented Mar 15, 2024

Uh oh!

vindarel commented Mar 15, 2024

Uh oh!

frejanordsiek commented Mar 15, 2024

Uh oh!

frejanordsiek commented Mar 16, 2024

Uh oh!

cxxxr commented Mar 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants