doom doctor resolution to fish shell issue does not remove warning

[dradetsky]

When running doom doctor I get this warning

  ! Detected Fish as your $SHELL
    Fish (and possibly other non-POSIX shells) is known to inject garbage
    output into some of the child processes that Emacs spawns. Many Emacs
    packages/utilities will choke on this output, causing unpredictable
    issues. To get around this, either:
    
      - Add the following to $DOOMDIR/config.el:
    
        (setq shell-file-name (executable-find "bash"))
    
      - Or change your default shell to a POSIX shell (like bash or zsh)
        and explicitly configure your terminal apps to use the shell you
        want.
    
    If you opt for option 1 and use one of Emacs' terminal emulators, you
    will also need to configure them to use Fish, e.g.
    
      (setq-default vterm-shell (executable-find "fish"))
    
      (setq-default explicit-shell-file-name (executable-find "fish"))

Now, I'm not totally sure this is a concern for me, or which resolution is appropriate, but just for fun I went ahead and added

(setq shell-file-name (executable-find "bash"))
(setq-default vterm-shell (executable-find "fish"))
(setq-default explicit-shell-file-name (executable-find "fish"))

to my user config. I would expect the warning to now disappear. But it doesn't.

I imagine this is because according to https://github.com/doomemacs/doomemacs/blob/master/docs/getting_started.org#load-order, $DOOMDIR/config.el is not loaded by the cli. I imagine this is by design, and I'm not totally sure what should be done about this other than to say that it seems suboptimal.

[dradetsky]

FWIW, one reason this was of interest to me was that I submitted a PR to projectile over another doom warning; see bbatsov/projectile#1916

I take for granted that the issue doom complained about is real, and thus that my fix is necessary (although if anyone knows otherwise please advise so we can save bbatsov the trouble). However, if the fix can't make doom stop complaining about the issue, that would kinda suck.

[hlissner]

I'm aware of the issue. Simply put, the fish test is done before Doom's doctor loads the user's config (which does include your config.el). However, I'm currently doing prep work for v3.0, and the entire doctor subsystem is undergoing a redesign, so I'll save resolving this until then.

[dradetsky]

Sounds good. It's not like I think this is an urgent issue. Just that it ought to be fixed eventually.