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

Two small improvements for named indexes U-Z #385

Merged
merged 6 commits into from
Oct 20, 2024
Merged

Conversation

muzimuzhi
Copy link
Collaborator

Currently,

  • Named indexes U-Z are supported in both from- and to-part of simple selectors like cell{-}{U-Z}={preto=x}, but only in to-part of child selectors odd/even. Thus cell{-}{even[2-Z]}={preto=x} is supported but cell{-}{even[X-Z]}={preto=x} is not.
  • The indexes converted from U-Z may be non-positive (hence invalid for following processes) in tables with small number of columns or rows.

This PR adds support for from-part of odd/even selectors and ensures the converted indexes are at least 1.

@muzimuzhi muzimuzhi changed the title Two small fixes for named indexes U-Z Two small improvements for named indexes U-Z Mar 18, 2023
@lvjr
Copy link
Owner

lvjr commented Mar 18, 2023

I think it will bring more and more troubles if a package starts to correct wrong user inputs: to me X is a wrong input if a table has only two rows and should cause errors.

@lvjr
Copy link
Owner

lvjr commented Mar 18, 2023

@muzimuzhi Could you help to confirm that l3build doesn't normalize end-of-line characters in .tlg files? It seems the eol characters are different on macos/linux.

@muzimuzhi
Copy link
Collaborator Author

muzimuzhi commented Mar 18, 2023

I think it will bring more and more troubles if a package starts to correct wrong user inputs: to me X is a wrong input if a table has only two rows and should cause errors.

It depends on how intelligent the language/package want to be. Named indexes U-Z are already part of that intelligence, and child selectors a family of another. For example python's slicing supports

>>> a = [1,2,3]
>>> a[-10:-1]
[1, 2]
>>> a[-10:10]
[1, 2, 3]

Currently

\documentclass{article}
\usepackage{tabularray}

\begin{document}
% `cell{-}{Y-Z}` works
\begin{tblr}{
  hlines, vlines,
  cell{-}{1-X}={preto=x}
}
  & & & \\
\end{tblr}

% `cell{-}{U-Z}` explodes
\begin{tblr}{
  hlines, vlines,
  % U = -6, Z = -1, four-column table
  cell{-}{U-X}={preto=x}
}
  & & & \\
\end{tblr}
\end{document}

throws low level error and the non-positive index is treated as zero. (In either way a fallback index is used.)

! Missing number, treated as zero.
<to be read again> 
                   ;
l.20 \end
         {tblr}

Thus I suggest to give a more helpful error message and (still) use 1 as fallback value.

Update: Then this seems to introduce a new message named index-out-of-bound which should be applied to several places with appropriate fallbacks. I don't want to introduce a big modification in current PR so if you don't like it, I can remove the resizing changes and leave the support for named indexes in from-part of odd/even.

@muzimuzhi
Copy link
Collaborator Author

muzimuzhi commented Mar 18, 2023

Could you help to confirm that l3build doesn't normalize end-of-line characters in .tlg files? It seems the eol characters are different on macos/linux.

l3build won't normalize EOL. The .tlg is generated on macOS and checked by the GitHub Actions workflow on Ubuntu and Windows. What's the concrete problem/difference you noticed?

Just guessing, git configs core.eol and core.autocrlf may help.

@lvjr
Copy link
Owner

lvjr commented Mar 18, 2023

l3build won't normalize EOL. The .tlg is generated on macOS and checked by the GitHub Actions workflow on Ubuntu and Windows. What's the concrete problem/difference you noticed?

See extra-002.tlg file in this pull request. It is quite annoying that the diff of this file is wrong.

@muzimuzhi
Copy link
Collaborator Author

l3build won't normalize EOL. The .tlg is generated on macOS and checked by the GitHub Actions workflow on Ubuntu and Windows. What's the concrete problem/difference you noticed?

See extra-002.tlg file in this pull request. It is quite annoying that the diff of this file is wrong.

It's OS specific. Windows uses CRLF while *-nix tends to use LF, see https://www.wikiwand.com/en/Newline#Representation. As I appended to #385 (comment),

Just guessing, git configs core.eol and core.autocrlf may help.

and GitHub also provides a setting to hide whitespace when reviewing.

I will try with git configs and perhaps push a commit to use CRLF.

@lvjr
Copy link
Owner

lvjr commented Mar 18, 2023

Just guessing, git configs core.eol and core.autocrlf may help.

I always turn off this autocrlf option in my local git repos, because I believe git should not touch these kind of things. It is a job of repo maintainers and build tools to decide and normalize eol characters.

But l3build is missing this feature. Maybe someday I will add some code to build.lua to normalize eol characters (always using LF characters) of tlg files. It seems doable.

@davidcarlisle
Copy link

you could open an issue at L3build

@muzimuzhi
Copy link
Collaborator Author

l3build won't normalize EOL.

I was wrong by using wrong patterns %r/%n yesterday. An issue is opened at l3build, latex3/l3build#293.

@muzimuzhi
Copy link
Collaborator Author

Just guessing, git configs core.eol and core.autocrlf may help.

I always turn off this autocrlf option in my local git repos, because I believe git should not touch these kind of things. It is a job of repo maintainers and build tools to decide and normalize eol characters.

Seems it's not always like this, for example front-end tools babel and npm both don't normalize EOL, see latex3/l3build#293 (comment).

EOL are special characters that are (almost) transparent to human and most modern programs that read in files, and perhaps only meaningful for softwares that compute file checksums or do file comparisons. So it's not too bad for git to touch EOLs.

@lvjr lvjr merged commit 6c65d61 into lvjr:main Oct 20, 2024
@lvjr lvjr added this to the 2025A milestone Oct 20, 2024
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