dvc
dvc copied to clipboard
iterative
[RFC] Use `--no-config` option in fish > 3.3
This is a first draft of fixing #1307. However, I am quite unhappy about it.
Because only fish version > 3.3 supports the --no-config option, it cannot be applied without checking first. Thus, I am running the shell and parse the $shellname_VERSION variable.
I went that way, because the $FISH_VERSION does not seem to be available in the dvc process context. Also, it is not guaranteed that dvc itself is run with $SHELL itself, is it?
I do not think, this is a great solution and would like some input. Does somebody have a better idea?
-
[x] ❗ I have followed the Contributing to DVC checklist.
-
[x] 📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here.
Thank you for the contribution - we'll try to review it as soon as possible. 🙏
Perhaps adding a config flag to dvc to manually enabling fish's --no-config option would be a cleaner solution.
@clamydo When is $FISH_VERSION available? We should be able to get it if you run dvc from fish.
When is
$FISH_VERSIONavailable? We should be able to get it if you run dvc from fish. I thought so too, however it is a somewhat magic unexported variable.
Also ZSH_VERSION and BASH_VERSION seem to be behave identical, i.e., unexported.
@clamydo Hey, any plans to continue working on this?
Hey @efiop, in principle yes. It's just my time is stretched thin, so expect slow progress. Sorry for letting you wait.
I also wouldn't mind, if someone else would take over.
@clamydo, could you please elaborate on what's missing/left here to do? It'd be easier for someone to take over.
Overall, the code looks good to me, but looking at it, it feels that the shell setup does not belong here at all, and we are mixing shell-setup/task-monitor/cache-restore, etc. in a place where the only thing it should be doing is "run" the script.
Also shell setup is a one time thing. I think what we eventually need is something like this (not suggesting to do this right now, just something to keep in mind).
