yazi: improve fish integration

[iSma]

Description

Improve yazi fish integration:

Calls yazi as command yazi, allowing to use "yazi" as shellWrapperName without infinite recursion. This could be backward-incompatible for users who have an alias named "yazi" and expect this shell wrapper to call that alias instead of the yazi executable directly.

Also defines the wrapper with programs.fish.functions instead of interactiveShellInit, which seems to be the preferred way.

Checklist

• Change is backwards compatible. (with one exception, see above)

• Code formatted with ./format.

• Code tested through nix-shell --pure tests -A run.all or nix develop --ignore-environment .#all using Flakes.

• Test cases updated/added. See example.

• Commit messages are formatted like

  {component}: {description}
  
  {long description}
  

  See CONTRIBUTING for more information and recent commit messages for examples.

• If this PR adds a new module

  • Added myself as module maintainer. See example.

Maintainer CC

@XYenon @eljamm

[eljamm]

This could be backward-incompatible for users who have an alias named "yazi" and expect this shell wrapper to call that alias instead of the yazi executable directly.

Could you explain this, perhaps? To me, it seems that we're doing the same thing: creating a function for the integration, but now we're storing it in functions instead of including it directly in the config.

[iSma]

This could be backward-incompatible for users who have an alias named "yazi" and expect this shell wrapper to call that alias instead of the yazi executable directly.

Could you explain this, perhaps? To me, it seems that we're doing the same thing: creating a function for the integration, but now we're storing it in functions instead of including it directly in the config.

Yes, that part doesn't change; the end result is that we create a function named $shellWrapperName.

The difference I was pointing to is the way yazi is called from within this function: I've changed it to command yazi which calls executable directly and ignores potential shell aliases named "yazi". Previously, a user could potentially define alias yazi=yazi --some-options and the wrapper would call this alias.

I don't think the old behavior is particularly useful, since users can (and probably should) set their options directly in the settings. But I still wanted to point it out for the sake of completeness.

[eljamm]

The difference I was pointing to is the way yazi is called from within this function: I've changed it to command yazi which calls executable directly and ignores potential shell aliases named "yazi".

Ah, I see. I missed that, somehow.

Since this might be a breaking change, do you think it's better if we write something in the news file, like this perhaps:

home-manager/modules/misc/news.nix

Lines 1679 to 1690 in fc52a21

	{
	time = "2024-06-26T07:07:17+00:00";
	condition = with config.programs.yazi;
	enable && (enableBashIntegration || enableZshIntegration
	|| enableFishIntegration || enableNushellIntegration);
	message = ''
	Yazi's shell integration wrappers have been renamed from 'ya' to 'yy'.
	A new option `programs.yazi.shellWrapperName` is also available that
	allows you to override this name.
	'';
	}

But just for fish integration users.

[iSma]

The difference I was pointing to is the way yazi is called from within this function: I've changed it to command yazi which calls executable directly and ignores potential shell aliases named "yazi".

Ah, I see. I missed that, somehow.

Since this might be a breaking change, do you think it's better if we write something in the news file, like this perhaps:

home-manager/modules/misc/news.nix

Lines 1679 to 1690 in fc52a21

	{
	time = "2024-06-26T07:07:17+00:00";
	condition = with config.programs.yazi;
	enable && (enableBashIntegration || enableZshIntegration
	|| enableFishIntegration || enableNushellIntegration);
	message = ''
	Yazi's shell integration wrappers have been renamed from 'ya' to 'yy'.
	A new option `programs.yazi.shellWrapperName` is also available that
	allows you to override this name.
	'';
	}

But just for fish integration users.

Yes to be on the safe side I added a news entry.

[eljamm]

Thanks, this looks good to me.

[rycee]

Thanks! Merged to master now 🙂