Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup character io interfaces #348

Closed
wants to merge 47 commits into from

Conversation

rmkaplan
Copy link
Contributor

I think this is ready for more general testing and eventual merging. It isolates the XCCS/NS external format dependencies into one new file XCCS, and all character access then goes through a few abstract functional interfaces (\INCCODE, \PEEKCCODE...), with the XCCS implementations still set up as the default.

This required touching and recompiling lots of files, but it all seems to hang together in this full sysout and in the LFG system built on top.

Other improvements/fixes to MODERNIZE, WHEELSCROLL, CLIPBOARD, and others.

rmkaplan and others added 30 commits May 8, 2021 23:39
Committing this branch for further testing.  I know at least that the TTY output stream somehow is defaulting to :XCCS, which is wrong, but I haven't yet found the interface for that.
No top-level calls to the NS specific functions, just to the generic \OUTCHAR etc.

Updated full.database
They can be dragged by their title bars
Also made spelling of default-externalformats consistent with FILEIO
EOL's printed as LF's will be read as EOL
meta,a maps to 1,a now.  But slowly propagating this to TEDIT, SEDIT, etc will make it easier to change the coding of meta characters, e.g. as part of a Unicode transition.
Minor cleanup, avoid typical user entry and APPLY*
Also sets up mappings in the \COMMANDKEYACTIONS, whatever that is
So that things like Masterscope don't break
Looks at the WHEREIS database, if present, for FNS and FUNCTIONS if it has no other information.  . WHO CONTAINS ANY CALLING FOO works, but not the inverse:  . WHO DOES FUM CONTAIN.  We still need to figure out why the CONTAINS table isn't populated
Now uses generic \OUTCHAR to get the proper function from the stream (or default)
Some of the macros weren't correct.
Cleaner separation between external \OUTCHAR and internal BOUT
#343
\TEDIT.BUTTONEVENTFN actually takes a second STREAM argument.  I don't see where it is ever called with that.  The modernize replacement binds that argument, but it isn't being passed to the original.
Mostly just qualifying references, more work to get BIGBITMAP stuff out of ADISPLAY and to eliminate ambiguity of LINE record (now XXLINE in XXGEOM)
Mostly new LCOMS where \OUTCHAR calls were compiled open
Old tools for reading wikipedia XCCS tables, sources/XCCS will deal with XCCS external format
… file

XCCS is the default, but can be swapped out (eventually) by setting a few variables, without recompiling everything
Copy link
Member

@masinter masinter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there were changes (introductions of functions) to BIGBITMAPS in the library -- not loaded in FULL. Were these not part of the IO functions?

Copy link
Member

@masinter masinter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHAT was recompiled with no changes. CHAT(SHELL) is the only useful CHAT target.
But shouldn't it assume :UTF-8 ? I think CHAT(host) could turn into ssh but for now it is better to leave out?
CHAT(SHELL) currently always opens /bin/csh (C-shell) which isn't in most Linux distros. See #121

@rmkaplan
Copy link
Contributor Author

rmkaplan commented Jul 22, 2021 via email

@rmkaplan
Copy link
Contributor Author

rmkaplan commented Jul 22, 2021 via email

@rmkaplan
Copy link
Contributor Author

rmkaplan commented Jul 22, 2021 via email

Copy link
Member

@masinter masinter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there was a change to SYSEDIT that removed resetting MSMACROPROPS to leave MACRO-FN on the list, leaving the value on MSANALYZE. Macros for all kinds are analyzed by analyzing their expansion. There was also a patch to remove / no longer generate spurious incorrect masterscope "templates" (call to SETTEMPLATE in GETTEMPLATE).
Otherwise,

@rmkaplan
Copy link
Contributor Author

rmkaplan commented Jul 22, 2021 via email

@rmkaplan
Copy link
Contributor Author

rmkaplan commented Jul 22, 2021 via email

Copy link
Member

@masinter masinter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with MSMACROPROPS is just an indication that you made your branch and didn't update it from master for months; I wasn't doing a lot of editing but we need to resolve the conflicts (not many).

I've gone through about 50 / 150 of the changed files in about 5 hours. I'll try to go through the rest.

@@ -1454,42 +1570,42 @@
DESWRD))
WIDTH MAP 0COLOR 1COLOR))
(COND
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason the font-change character changed in a way that makes these changes show up in diffs

@rmkaplan
Copy link
Contributor Author

rmkaplan commented Jul 23, 2021 via email

Copy link
Member

@masinter masinter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while there are a lot of "cosmetic" issues (bad pretty printing, edit comments lost, etc) there are a few more serious issues. I think it's necessary to sort out the "Masterscope gives error on ambiguous record field" changes from the "support external character encoding". The former could be fixed by adding a per-file record priority list, removing the necessity of any source edits. And the READTABLE should be independent of the file encoding. READ works on a stream of characters, which remain the same character independent of whether they are encoded in XCCS, UTF-8, ISO8059-10 or what have you.
I propose using this branch as a model, and bringing forward changes one at a time starting with current master.

(DECLARE%: EVAL@COMPILE DONTCOPY
(DECLARE%: EVAL@COMPILE

(RPAQQ UP 156)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm wary about using such common words for constants. WS-UP, WS-DOWN etc?


(DEFMACRO WITH-READER-ENVIRONMENT (ENV . BODY)
`((CL:LAMBDA (E)
(LET ((*PACKAGE* (ffetch (READER-ENVIRONMENT REPACKAGE) of E))
(*READTABLE* (ffetch (READER-ENVIRONMENT REREADTABLE) of E))
(*READ-BASE* (ffetch (READER-ENVIRONMENT REBASE) of E))
(*PRINT-BASE* (ffetch (READER-ENVIRONMENT REBASE) of E)))
(*PRINT-BASE* (ffetch (READER-ENVIRONMENT REBASE) of E))
(*EXTERNALFORMAT* (ffetch (READER-ENVIRONMENT REFORMAT) of E)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where are the other variables declared SPECIAL? Why just the external format? Is this really at the right level? The other elements don't depend / vary when you change XCCS and UTF-8... they're EXTERNAL-FORMAT independent

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue #371

@rmkaplan
Copy link
Contributor Author

rmkaplan commented Jul 23, 2021 via email

@rmkaplan
Copy link
Contributor Author

rmkaplan commented Jul 23, 2021 via email

@masinter
Copy link
Member

Can we at least (a) implement some "unit tests" that makes sure the UTF-8 code is working? Preferably something that can be automated?
(b) agree to make up "issues" that cover the problems I identified, as a way of getting agreement on what problems you agree need fixing and why?
I didn't finish reviewing all of the files, and those that diff well (because of pretty printing) are an issue

@masinter
Copy link
Member

no longer needed; was turned to draft after merging to make sure issues raised / tested

@masinter masinter closed this Aug 17, 2021
@masinter masinter deleted the Cleanup-character-IO-interfaces branch August 17, 2021 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants