PR#5: fix nano-default-editor.fish - don't give EDITOR an universal scope - rpms/nano

rpms / nano

#5 fix nano-default-editor.fish - don't give EDITOR an universal scope

Closed 3 years ago by kdudka. Opened 3 years ago by zdohnal.

Unknown source master into master

fix nano-default-editor.fish - don't give EDITOR an universal scope

Zdenek Dohnal • 3 years ago

b4fff48

nano-default-editor.fish

file modified

+1 -1

		`@@ -4,5 +4,5 @@`
		`# Cf. https://fishshell.com/docs/current/index.html#variables-scope`

		`if ! set -q EDITOR;`
		`- set -Ux EDITOR /usr/bin/nano`
		`+ set -x EDITOR /usr/bin/nano`
		`end`

nano.spec

file modified

+18 -1

		`@@ -1,7 +1,7 @@`
		`Summary: A small text editor`
		`Name: nano`
		`Version: 5.3`
		`- Release: 3%{?dist}`
		`+ Release: 4%{?dist}`
		`License: GPLv3+`
		`URL: https://www.nano-editor.org`
		`Source: https://www.nano-editor.org/dist/latest/%{name}-%{version}.tar.xz`
		`@@ -77,6 +77,20 @@`
		`install -Dpm 0644 %{SOURCE12} %{buildroot}%{_sysconfdir}/profile.d/%{basename:%{S:12}}`
		`install -Dpm 0644 %{SOURCE13} %{buildroot}%{_datadir}/fish/vendor_conf.d/%{basename:%{S:13}}`

		`+ # We need to manually remove EDITOR set by a previous versions`
		`+ # and let it be set with the updated nano-default-editor.fish.`
		`+ # It was set with -U, which sets EDITOR to /usr/bin/nano even if`
		`+ # nano-default-editor is removed.`
		`+ # Remove after release or two...`
		`+ %post default-editor`
		`+ if [ test "$EDITOR" = "/usr/bin/nano" ]`
		`+ then`
		`+ if [ test "$SHELL" = "/usr/bin/fish" ] \|\| [ test "$SHELL" = "/bin/fish" ]`
		`+ then`
		`+ set -e EDITOR`
		`+ fi`
		`+ fi`
		`+`
		`%files -f build/%{name}.lang`
		`%doc AUTHORS COPYING ChangeLog INSTALL NEWS README THANKS TODO`
		`%doc build/doc/sample.nanorc`
		`@@ -95,6 +109,9 @@`


		`%changelog`
		`+ * Thu Oct 15 2020 Zdenek Dohnal <zdohnal@redhat.com> - 5.3-4`
		`+ - fix nano-default-editor.fish - don't give EDITOR an universal scope`
		`+`
		`* Mon Oct 12 2020 Neal Gompa <ngompa13@gmail.com> - 5.3-3`
		`- Ensure default-editor subpackage is easily swappable`

zdohnal commented 3 years ago

Hi,

the current fish profile for nano overrides EDITOR environment variable permanently even if nano-default-editor is removed.
The reason of this is -U parameter of set, which gives EDITOR to universal scope, which means it survives reboot (and it seems even uninstallation of original package).
The patch removes the option and fixes upgrades from older releases within %post scriptlet, because the way how to make EDITOR configurable again is to manually erase the variable. The scriptlet is applied only when EDITOR contains /usr/bin/nano and SHELL`` isfish```.

Would you mind adding the PR to the project? Please let me know if I should change something.

Thank you in advance!

Zdenek

kdudka commented 3 years ago

Hi Zdenek,

thank you for creating the merge request! I like the fix ofnano-default-editor.fish. However, the %post scriptlet could hardly do what you expect. set -e would erase the variable for the user that runs it, which will always be root when invoked with rpm. Another problem is that rpm will use bash to run %post even if the root user configured fish as their login shell.

ngompa commented 3 years ago

We probably don't need the scriptlet and can just drop the -U property and make sure F33 GA includes that fix.

I guess I misunderstood how universal variables work. I was trying to make it trivially overridable by enforcing it at the lowest level so higher levels would override it.

zdohnal commented 3 years ago

@kdudka I'm sorry I didn't take that into considerations :( . Please feel free to remove that %post default-editor scriptlet.

@ngompa would you mind taking care of getting the fix into F33? IIRC we are in Final Freeze and builds which must arrive into compose needs a special flag (probably GA blocker?).

kdudka commented 3 years ago

Fixed in nano-5.3-4.fc33: https://src.fedoraproject.org/rpms/nano/c/73a88e37

@ngompa Please try to get the build into Fedora 33 GA, so that fish users do not have to fix it themselves.