Page MenuHomeSolus

Update nano to 5.9 and freshen up nanorc
AbandonedPublic

Authored by abdulocracy on Oct 14 2021, 11:10 AM.

Details

Reviewers
JoshStrobl
DataDrake
Group Reviewers
Triage Team
Summary

Along with the upstream changes, this update includes better defaults for syntax highlighting.

Changelog:

  • The extension of a filename is added to the name of a corresponding temporary file, so that spell checking a C file, for example, will check only the comments and strings (when using 'aspell').
  • The process number is added to the name of an emergency save file, so that when multiple nanos die they will not fight over a filename.
  • Undoing a cutting operation will restore an anchor that was located in the cut area to its original line.
  • When using --locking, saving a new buffer will create a lock file.
  • Syntax highlighting for YAML files has been added.

Full changelog can be found here

Test Plan

Edit a couple files, see the pretty colors.

Diff Detail

Repository
R2131 nano
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 2014
Build 2014: arc lint + arc unit

Event Timeline

abdulocracy created this revision.Oct 14 2021, 11:10 AM
abdulocracy requested review of this revision.Oct 14 2021, 11:10 AM

Fix rundep.

JoshStrobl requested changes to this revision.Oct 15 2021, 9:40 AM
JoshStrobl added a subscriber: JoshStrobl.

Your stack is reversed.

This revision now requires changes to proceed.Oct 15 2021, 9:40 AM
abdulocracy requested review of this revision.Oct 15 2021, 10:59 AM

There is going to be a lot of duplication between nano-syntax-highlighting and nano itself when it comes to its nanorc files, which begs the question if we should be removing the nanorc files from nano entirely since they're almost certainly going to be superceded by nano-syntax-highlighting.

There are a few that nano-syntax-highlighting doesn't cover, and also, scopatz's files haven't seen a release in a while, so some of nano's files are better ATM. Constantly choosing which ones would be better OOTB is a nightmare so we could just go with scopatz as default and let users choose if they want.

JoshStrobl added a comment.EditedOct 15 2021, 1:27 PM

There are a few that nano-syntax-highlighting doesn't cover, and also, scopatz's files haven't seen a release in a while, so some of nano's files are better ATM. Constantly choosing which ones would be better OOTB is a nightmare so we could just go with scopatz as default and let users choose if they want.

Yea I think I'd prefer that (having nano-syntax-highlighting enabled by default). Saves having both enabled.

abdulocracy added a comment.EditedOct 15 2021, 2:57 PM

On further thought, I'm not sure if including scopatz's as default is a good idea. There hasn't been any activity for nearly a year, and after numerous nano updates, some of the files are buggy (even nanorc.nanorc).
Seems it's best to include the scopatz files with lower precedence, so we have up-to-date definitions from nano and scopatz fills in definitions missing upstream (as hinted to in their readme).

Give scopatz definitions lower precedence.

Wonder if it might be worth playing with that --lite flag they have in scopatz. In my mind it makes sense to use the upstream nanorc if available and then supplement with scopatz. The main thing I was hoping to see us fix was vendoring scopatz in our own package sources.

abdulocracy added a comment.EditedOct 15 2021, 10:20 PM

The current state of the patch does just that, complimenting the upstream nanorc files, by including the scopatz files higher up than the upstream files.

Those aren't the same thing. With the current state of this patch both sets of syntax are always fully read in and parsed. The nanorc language doesn't assume that files are separate entities, they are more like scripts which get executed in order of inclusion. If you have two different files containing a syntax with the same name, the one read second takes precedent, but nano will have already read the previous syntax definition into memory. With --lite the files don't get installed at all in scopatz which means that only one syntax is read in instead of two.

Yes they do, with the --lite option all that happens is the script includes the scopatz definitions higher up than the upstream ones. All of the scopatz definitions are still included.

Prevent having to include duplicate definitions.

Remove useless line in package.yml.

This prevents the duplication, but we do get a very long list of includes. What do y'all think?

JoshStrobl added a comment.EditedOct 16 2021, 11:33 AM

I don't feel comfortable with us making these modifications during build time. I would rather this be a separate script that gets run by the packager, echos out the content with a note to "replace all contents in our nanorc after # xyz comment where everything after this is generated" sort of thing. That way the changes can be audited during package updates or be done independently when nano-syntax-highlighting is updated.

Would also be nice to have to either be provided a separate list of where each nanorc is coming from, or for the changes to our files/nanorc be limited in this patch to just those syntax changes, otherwise it is harder to audit all the changes.

Sorry, I gave them more credit than I should have and didn't read through the script to see what it was doing. My point still stands that having both get imported is problematic.

I don't think we should be modifying the nanorc at all during build time. It should be static. And don't think we should be using individual includes for different syntax files either. What I'd like to see is:

  1. Our vendored nanorc gets updated
    • To the latest upstream example
    • Without the modifications we had previously
    • With a single line to include the syntax files from nano
    • With a single line to include the scopatz directory
  2. Duplicate nanorc files in scopatz be removed entirely from the install when they are present in nano itself
  3. A note in this repo to always make sure scopatz is modified when new nanorc files are added or removed from nano
  1. Duplicate nanorc files in scopatz be removed entirely from the install when they are present in nano itself
  2. A note in this repo to always make sure scopatz is modified when new nanorc files are added or removed from nano

I thought about the same thing, but that would mean we ship the scopatz package tainted and people who might prefer the definitions there would be unable to switch to them easily.

I guess it is easy enough to fetch from GitHub though, if they really do choose to use them.

DataDrake requested changes to this revision.Jan 14 2022, 7:58 PM

@abdulocracy Lemme take some time after sync to think about what I requested and find a happy medium.

package.yml
11

should be nanorc now.

This revision now requires changes to proceed.Jan 14 2022, 7:58 PM

@abdulocracy, I've had some time to think it over and I think the integration bits are fine as-is, but this will need a rebase on the newer nano package.

Staudey abandoned this revision.Aug 1 2022, 6:12 PM
Staudey added a subscriber: Staudey.

Dealt with in D13410 instead.

Thanks for the work, @abdulocracy!