Switch to getopt for command line parsing #169

augiedoggie · 2026-01-24T04:33:47Z

Fixes #144

augiedoggie · 2026-01-24T05:06:02Z

There is an intermittent bug when calling RefsReceived() right after Init(). I'll have to work around that.

augiedoggie · 2026-01-24T07:11:12Z

The intermittent bug seems unrelated to my changes since I can reproduce it on my other computer without these changes.

I like this version better because Koder always forks to the background unless --wait is used. Without this change, the first instance of Koder will always block even without --wait.

Reading data from stdin and sending it to the app might be complex and require a shared memory buffer or something, but that was already the case before this change.

KapiX · 2026-01-24T10:24:30Z

src/main.cpp

+			default:
+				_PrintUsage();
+				return 1;
+				break;


Unnecessary break

augiedoggie · 2026-01-24T22:55:08Z

I may end up closing this pull request and thinking about the problem more. I keep running into issues with whichever method I use. The most recent version is broken when being launched by Tracker and other apps.

It's possible Koder may need a small companion app to handle some command line options, similar to Pe/lpe.

KapiX · 2026-01-25T10:16:52Z

I like this version better because Koder always forks to the background unless --wait is used. Without this change, the first instance of Koder will always block even without --wait.

What is the problem with this behavior? It's predictable - you want it to wait you use -w. EDIT: Unless I misunderstood and it currently blocks the first time - that's not how I remember coding it though.

It's possible Koder may need a small companion app to handle some command line options, similar to Pe/lpe.

I don't like the companion app solution because it's not very discoverable.

Can you document the issues you ran into here so it can be taken into account when changing this part of code.

augiedoggie · 2026-01-25T20:18:15Z

What is the problem with this behavior? It's predictable - you want it to wait you use -w. EDIT: Unless I misunderstood and it currently blocks the first time - that's not how I remember coding it though.

Yes, it currently blocks the first time when --wait is not used.

Can you document the issues you ran into here so it can be taken into account when changing this part of code.

The short version is that it's difficult to fork the first instance of a BApplication into the background when launched by command line, while still allowing it to have access to any initial B_REFS_RECEIVED message when launched by GUI. I had a small conversation with waddlesplash in IRC to see if I could find a way around the problem.

12:39:40    augiedoggie | waddlesplash: do you know... if an app is launched with a refs received message, can the app get that message from main() before the
                        | BApplication is created?
12:40:00  @waddlesplash | off the top of my head I don't know
12:40:19  @waddlesplash | I never really looked into the mechanisms there
12:40:21    augiedoggie | can i fork before creating a BApplication and still have the message passed on?
12:41:07    augiedoggie | i think i tried that and it didn't work for some reason
12:43:15  @waddlesplash | a glance at the code, and it might depend on pre-registration
12:43:33  @waddlesplash | the parent after a fork will have the same team ID so will still be pre-registered
12:43:38  @waddlesplash | the child won't
12:43:39    augiedoggie | :/
12:43:46  @waddlesplash | what's the usecase?
12:44:11    augiedoggie | trying to get Koder to fork to the background consistently when it's started from the command line
12:44:33  @waddlesplash | well on the command line it doesn't get a refs message?
12:44:48    augiedoggie | right
12:45:24  @waddlesplash | I think realistically we need an "lkoder" or equivalent tool like "lpe"
12:45:25    augiedoggie | i can get it to work fine from the command line by using roster to launch the application in the background but the refs received
                        | message isn't passed on
12:45:36  @waddlesplash | "lpe" has the nice property that it doesn't quit until you close the Pe window that it opened
12:45:49  @waddlesplash | makes it suitable for use as an editor for various things like git
12:45:52    augiedoggie | heh, i mentioned that in the pull request and he's not fond of the companion app solution
12:46:01  @waddlesplash | well, I don't know any other way to do this
12:46:05    augiedoggie | yean :P
12:46:06  @waddlesplash | besides maybe a special switch
12:46:29    augiedoggie | i did the special switch thing but i still need a way to forward the initial bmessage
12:47:22    augiedoggie | right now Koder does have a --wait option which works ok-ish
12:47:58    augiedoggie | it's more of a personal annoyance that the first instance of the app will always block from the command line regardless of that switch
12:48:33  @waddlesplash | yes I agree
12:48:46  @waddlesplash | fork only on isatty()?
12:49:00    augiedoggie | i had that thought too but hadn't investigated it

augiedoggie · 2026-01-26T01:23:54Z

I still need to do some testing and cleanup but the latest version works well. I would have preferred to do things a bit differently but I think it's acceptable.

Fixes KapiX#144

KapiX

About companion app: I'd agree to support it if this was a generic app. I'm pretty sure other apps might want to capture stdin or stdout and support blocking when launched from command line, so extracting it out of Pe and providing some kind of common protocol might make sense. Users could then add aliases like lpe (or those could be provided by the app) to their shell for each app.

KapiX · 2026-01-26T22:26:45Z

src/App.cpp

 App::ReadyToRun()
 {
-	if(CountWindows() == 0 && fSuppressInitialWindow == false) {
+	if(fSuppressInitialWindow == false && CountWindows() == 0) {


Reversing the order is meaningful here?

At first this was done accidentally because I removed it in one change and then put it back in a later change. Technically this would be more efficient because the first test is a simple boolean variable comparison and so it wouldn't need to proceed with a function call and whatever else CountWindows() does.

KapiX · 2026-01-26T22:27:08Z

src/App.cpp



-void
-App::ArgvReceived(int32 argc, char** argv)


What's the reason behind removing this?

I think handling command line arguments in two places complicates the startup process and possibly introduces code duplication, so it would be nice to do it in one place or the other. If the arguments are processed inside main() then we can do validity checks on the arguments before we even create the BApplication object. This makes it easier to return proper exit codes when run from the command line. It can also prevent the situation where you create the BApplication, have errors during ArgvReceived, and have to destroy/Quit() the BApplication right away if there are no other windows.

KapiX · 2026-01-26T22:28:53Z

src/main.cpp

+void
+_PrintUsage()
+{
+	std::cerr


When this is output from --help it should be printed to stdout (std::cout). If it's a result of unknown option or other error case it should go to std::cerr.

KapiX · 2026-01-26T22:32:30Z

src/main.cpp

+_PrintUsage()
+{
+	std::cerr
+		<< "Usage: Koder [options] file..." << std::endl


Using std::endl is doubly wrong here. std::endl besides adding a newline also flushes the stream but:

std::cerr is unbuffered and will flush after any output that is sent to it.

Flushing after every newline usually only degrades performance, with no benefit.

Flushing is useful if you are outputting text before very long operation (the OS might not show the output without flush until after the operations ends).
Change endls to "\n".

Ok, I can't remember if I usually use endl of '\n' in my own apps, it's probably a 50/50 split.

Of course, if you'd like I can switch back to the c-style fprintf too, don't hesitate to ask for it.

No, it's good to change to C++.

KapiX · 2026-01-26T22:33:23Z

src/main.cpp

+		return 0;
+	}
+
+	int option_index = 0;


Suggested change

int option_index = 0;

int optionIndex = 0;

KapiX · 2026-01-26T22:37:30Z

src/main.cpp

+					return 0;
+				}
+				return 1;
+			} break;


redundant break

KapiX · 2026-01-26T22:37:39Z

src/main.cpp

+			case 'h': {
+				_PrintUsage();
+				return 1;
+			} break;


redundant break

KapiX · 2026-01-26T22:41:56Z

src/main.cpp

+	const char* args[] = { "--launchkoderapp", nullptr };
+	// Use the entry_ref version of Launch() to make sure B_SINGLE_LAUNCH works correctly
+	status_t status = roster.Launch(&appRef, 1, args, &team);


Suggested change

const char* args[] = { "--launchkoderapp", nullptr };

// Use the entry_ref version of Launch() to make sure B_SINGLE_LAUNCH works correctly

status_t status = roster.Launch(&appRef, 1, args, &team);

auto args = std::to_array<const char*>{ "--launchkoderapp", nullptr };

// Use the entry_ref version of Launch() to make sure B_SINGLE_LAUNCH works correctly

status_t status = roster.Launch(&appRef, args.size(), args.data(), &team);

src/main.cpp

KapiX · 2026-01-26T22:44:07Z

src/main.cpp

+				return 1;
+			} break;
+			case 0: {
+				if(strcmp(long_options[option_index].name, "launchkoderapp") == 0) {


What's the use case for this option?

Yeah, I was trying to avoid the "secret switch" thing. To be consistent, Koder should always fork to the background unless --wait is used. So, we can use BRoster::Launch to do that, but we need to tell the newly launched copy of the application that it should start the BApplication process and prepare to receive one of the three message types below.

In some ways this is similar to the isatty() == 0 situation except that when running from the command line, the newly launched copy may have a stdout file descriptor.

augiedoggie force-pushed the getopt branch from 6063b74 to abda831 Compare January 24, 2026 04:41

augiedoggie marked this pull request as draft January 24, 2026 05:05

augiedoggie force-pushed the getopt branch 2 times, most recently from 1234773 to e596938 Compare January 24, 2026 07:02

augiedoggie force-pushed the getopt branch from e596938 to 5679e14 Compare January 24, 2026 07:28

KapiX reviewed Jan 24, 2026

View reviewed changes

src/main.cpp Outdated

default:

_PrintUsage();

return 1;

break;

Copy link

Owner

KapiX Jan 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary break

augiedoggie force-pushed the getopt branch from 5679e14 to c928e4d Compare January 26, 2026 01:20

augiedoggie force-pushed the getopt branch from c928e4d to a1de65b Compare January 26, 2026 02:45

Switch to getopt for command line parsing

f66292d

Fixes KapiX#144

augiedoggie force-pushed the getopt branch from a1de65b to f66292d Compare January 26, 2026 03:01

KapiX requested changes Jan 26, 2026

View reviewed changes

Switch to getopt for command line parsing #169

Are you sure you want to change the base?

Switch to getopt for command line parsing #169

Conversation

augiedoggie commented Jan 24, 2026

Uh oh!

augiedoggie commented Jan 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

augiedoggie commented Jan 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

augiedoggie commented Jan 24, 2026

Uh oh!

KapiX commented Jan 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

augiedoggie commented Jan 25, 2026

Uh oh!

augiedoggie commented Jan 26, 2026

Uh oh!

KapiX left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KapiX Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

augiedoggie commented Jan 24, 2026 •

edited

Loading

augiedoggie commented Jan 24, 2026 •

edited

Loading

KapiX commented Jan 25, 2026 •

edited

Loading

KapiX left a comment •

edited

Loading

KapiX Jan 26, 2026 •

edited

Loading