I would have looked at adding this directly to Workspaces, but the comment above about not wanting GUI apps to also have command-line functionality dissuaded me.
I am actually calling in to Workspaces to do all the actual switching anyway, so really this logic belongs in Workspaces as long as it has command-line functions.
I would just integrate your changes into Workspaces directly. It doesn’t really make sense to me to divide that functionality into two apps.
We already have and use a couple of applications that offer command line functionality. Since Workspaces are very tightly related to the GUI session, it should just offer that same functionality from the command line, too IMO.
I certainly agree that splitting the functionality doesn’t make much sense.
The sensible choice is to either migrate the command-line interface to workspace into a dedicated tool or to just expand the functionality of Workspaces.
Is @nephele right that there’s a goal of not having command-line functionality in GUI apps? I have no idea; it doesn’t seem that everyone in this thread with a “Developer” tag on their account agrees.
Ugh.
Code-wise, my little app now duplicates all of the command-line functionality that I know of in Workspaces (and does it without spawning sub-processes anymore), so going in either direction is pretty simple.
I don’t want to waste anyone’s time with a PR that does one thing, if reviewers are going to want the other.
I think it is unreasonable, There are different expectations for gui apps, fpr example the single launch app will effecticely ignore commandline arguments if you run the app while it is already running. There are good reasons to keep this functionality seperate, which is also why screenmode exists and isn’t just Screen prefs
(Note that the original problem statement was beeing able to view the activate workspace on the commandline, which requires the command to provide output back on the commandline)
The tag means that we have full commit access, that means beeing able to push directly to haikus repo aswell as accept or veto code review submissions. Of course we don’t always agree on stuff and do discuss things openly
Fair enough; unless it’s in the documentation, I wouldn’t expect all y’all to agree on everything anyway. I just don’t want to waste anyone’s time by going the wrong way.
So, I’ll set up to submit it as “/haiku/src/bin/workspacesctl.cpp” and we’ll see what reviewers have to say.
Well, let’s just say that I disagree here, and since Workspaces already has a command line interface (like other apps already have), why not put it there in addition?
There is no reason it would have to ignore its command line.
It can’t not do that though? Unless I am misunderstanding here having a single launch flag means an app will not be started again by the roster, so will never get the tty context for the second invocation on the commandline. Working around this seems needlesly complex, it also avoids having to check in every App if it is appropriate to open a window now since you could have launched it only for it’s commandline mode. If you split between a cli and app you have a clear distinction and don’t have to split this up, we can also document this better, the list of cli apps is used to figure out which apps can be used for scripting like this, but it does not list any gui apps that happen to supports some argv messages, only commandline tools with names.
Sometimes it feels like it takes longer to decide how something should be done than it does to just do it.
Since I’m not a project dev, and am totally new around here too, I don’t feel comfortable pushing for either approach. On my own, I’d probably opt for a policy of not putting command-line functionality in GUI apps but I don’t feel that strongly about it.
Points in favor of just adding functionality to Workspaces:
it already has part of the functionality
it already has a mechanism for knowing when it’s invoked for its command-line use vs. GUI use
Axel is a credited author on it and wants it that way
Points in favor of adding a new utility program:
separation of GUI apps and command-line tools makes it easier to figure out what will have command-line utility (apps/ vs. bin/)
other apps/ generally don’t offer command-line functionality?
It’s already done as a standalone, but it wouldn’t be that hard to fit into the current app’s arg parsing either so ease of implementing isn’t much of a factor.
I don’t really want to go through setting up a review without an agreement between y’all on how I should proceed first? Wouldn’t that just end up with this debate continuing on the review?
Probably, but others would probably chime in. Anyway, since there are arguments for both sides, just implement it how you prefer to do it. I don’t think it will be stalled either way.
The roster only starts applications when you launch them by signature. Otherwise, you have full control over what happens when and how.
Anyway, I could accept the argument of separating functionality. But since Workspaces already processes CLI arguments, putting it elsewhere seems wasteful. If we want to go down the route of separation, we should remove that functionality in Workspaces, and do the same in other apps like the Time preferences app.
I’ll look through the apps/ directory to see how many of them:
take command line args
do something non-GUI-related with those args
do something with them that would be difficult to extract or would duplicate lots of code if moved to a standalone utility
If it’s just a few, then it probably makes sense to have a general guideline that command-line functionality should be separate from GUI functionality and should be in distinct programs in /bin (maybe named with the lower-case version of the app’s name: apps/Workpsaces pairing with bin/workspaces, for example).
If there are a lot, or if there are any with difficult-to-migrate code in them, then it’s probably best to just say that horse is out of the barn and bundle command-line support into the GUI apps and just document it better.
OK, that horse has well and truly bolted and is somewhere in the next county; it’s totally normal for Haiku GUI apps to expose significant functionality to the command line.
DeskCalc, for example, offers a full calculator to the command line. RemoteDesktop offers a remote command execution service to the command line.
In all, 61 .cpp files in src./apps use ‘argv’ in some way:
So, it would be a significant task to implement a policy that GUI apps shouldn’t also try to be command line apps; if anything, the policy so far appears to have been that they should be.
So I think it’s probably best to say that Haiku values not duplicating code over cleanly separating GUI and command-line functions and just try to do a better job of documenting it when it happens. Workspaces, for example, doesn’t list all of its navigation arguments in its usage statement.
So I’ll add the other navigation commands to the existing set in Workspaces and make sure they’re all listed in its ‘–help’ output.
I don’t think “it used to be like this” is a normal concern for such matters, if we do want to seperate such things out we will just do so
We just need a consensus for it, you will also find a lot of non-layout code in the sourcetree that we gradually remove.
Also note that doing something with argv does not indicate a cli interface at all. Gui apps can receive files to work with as “App filepath” and get that in a BMessage named similarily, but that isn’t really handeling argv directly. Specifically the “Argvreceived” you greoped for is in a context where the app can get a single line from the commandline but is unable to respond to it on the commandline (and iirc also unable to influence the exit code the called sees)
Yeah, past usage is no guarantee that it’s the plan.
But I think it’s widespread enough that it’s obviously the current common practice; changing it would be a significant effort and would break any user expectations or scripting that’s based on the current practice.
So I’d think changing it should only happen with a general consensus of the dev team that it’s what they want and worth the effort?
As an aside: note that that part of the command was a ‘grep -v’; I was excluding ArgvReceived.
I also excluded “main”, since both of those contexts would have ‘argv’ without necessarily using it.
It looks like more input from developers is expected, so here you go: in this case I agree with axeld, let’s improve the existing command line support in the existing workspaces app
I tried setting up an account on the review site before I noticed in the doc that it says that only login-by-github works there, so that wasted some of my time because even though the account creation seems to have worked I get a lovely haiku error message when I try to login.
Today, I tried signing in with my github account and accepted its offer to link that to the account I’d already created. This did not go well and was probably not a good idea.
I’ll go re-try all this from a different browser and see if that behaves any differently.