search

karli-sjoberg / gswitch

BSD 3-Clause "New" or "Revised" License
91 stars 14 forks source link

Stop reloading when in non-graphical target #3

Closed ewagner12 closed 5 years ago

ewagner12 commented 5 years ago

Previous version would automatically reload to default (usually graphical) target when booting or manually switching. This caused problems when trying to boot to a non-default target such as a multi-user text console. This is updated so that the user is informed switching will not be done when in the non-default (usually text console or some such) target. Open to ideas on if/how this could be done better.

karli-sjoberg commented 5 years ago

Hey man!

I have no problems with the pull request as such, I just gotta ask though, is this really an issue? It seems unlikely to me that people are using gswitch without graphical target. What's the scenario here?

ewagner12 commented 5 years ago

Yeah I agree that its not at all a common thing. The situation I found where I needed it was after a bad bios flash to my gpu. It was crashing before login and I needed to go to a non-graphical target while the egpu was connected to fix it, but the boot script kept reloading to the graphical target. Maybe a more simple fix would be to just change line 8 in gswitch.service to WantedBy=graphical.target That shouldn't affect the vast majority of users who are booting into the graphical target anyway, but it still ensures the boot script won't be triggered when in a non-graphical target.

karli-sjoberg commented 5 years ago

@ewagner12 OK, cool! Just as long as we're not "fixing" imaginary issues, I'm totally fine with it :+1:

Maybe a more simple fix would be to just change line 8 in gswitch.service...

I like that! If that works, it would be so much simpler! How about you test that out first and report back?

And if it doesn't work, we'll just go back to your original approach. I must warn you though, I won't accept this without some style-improvements :stuck_out_tongue_winking_eye:

Nah, I won't be all that hard on you, but I'm going to insist on 80 character width max. And with your suggestion, you list all loaded targets and egrep after common ones, but who's to say the "best" target is the "highest" (head -1) in that list?

Also, don't ask why, but on one of my servers, the default target is actually graphical.target despite it having no X running, so your logic probably needs some tuning if we go down this route. Other than that, I think it looks good, no problemo!

ewagner12 commented 5 years ago

Haha I'm notorious for using a 5 line solution when a 1 liner would work. The change to just gswitch.service works for me after deleting the old symlink and reenabling the service so that's definitely the way to go.

karli-sjoberg commented 5 years ago

@ewagner12 Awesome! So let's see a highly simplified commit so I can slam dunk this sucker! :smile:

karli-sjoberg commented 5 years ago

Thanks man! Awesome!