Merge remote-tracking branch 'upstream/main' into dev

Author: effigies

.github/CONTRIBUTING.md

@@ -161,10 +161,101 @@ git push -u origin feat/short-desc
 
 Open PR, set base branch to `dev`.
 
+### Conventions for branches, commits and changelogs
+
+The following conventions are intended to make contributions legible to other contributors
+and, ultimately, users.
+They are also there to help you, as a contributor, focus on doing one task at a time.
+
+The following are common types of contributions, sorted roughly by order of interest
+to the reader:
+
+| Contribution type | Branch prefix | Commit prefix | Changelog section       |
+|-------------------|---------------|---------------|-------------------------|
+| Bug fix           | `fix/`        | `fix:`        | Fixed                   |
+| Feature           | `feat/`       | `feat:`       | Added                   |
+| Documentation     | `doc/`        | `doc:`        | Added or Infrastructure |
+| Refactor          | `rf/`         | `rf:`         | Changed                 |
+| Testing           | `test/`       | `test:`       | Changed                 |
+| Style-only        | `sty/`        | `style:`      | Infrastructure          |
+| Maintenance       | `chore/`      | `chore:`      | Infrastructure          |
+
+If you are contributing a bug-fix, it can help focus your efforts to name your branch
+`fix/problem-with-X`, and start commit messages where you write fixes with `fix:`.
+Similarly, branches and commits that focus on features, documentation, refactors,
+and so on, all have prefixes that help your reader understand what you were trying to do.
+
+If you find your contribution spanning more than one of these sections,
+it might be worth breaking into multiple pull requests.
+Exceptions include tests and documentation relevant to the main thrust of the PR,
+which are always welcome.
+
+None of this is set in stone, and we all make mistakes, so don't worry about
+following it to the letter.
+We will let you know if your contribution is doing too many things and work with
+you to split it into more manageable pieces.
+
+#### Changelog entries
+
+We are using the [scriv] changelog management tool to help keep our changelog organized.
+
+Most pull requests will deserve changelog entries, and some may deserve more than one.
+Scriv allows changelog entries to be made as part of the pull request, which gives
+the contributor and the reviewer a chance to consider the impact of the pull request.
+
+To add an entry, run:
+
+```
+scriv create --edit
+```
+
+(If you're wondering how to install `scriv`, we recommend using [uvx scriv][].)
+
+Your editor will open a Markdown file with commented sections.
+Find the appropriate section(s) for your contribution and write a 1-3 line description.
+
+#### Style contributions
+
+Style changes in code can be a source of frustration.
+Some editors will reformat entire files when a single line is changed,
+swamping the meaningful change with a large number of cosmetic changes.
+
+We have provided a [`.editorconfig`](https://editorconfig.org) file in the repository,
+which should be respected by [most editors](https://editorconfig.org/#pre-installed),
+and there are plugins available for [others](https://editorconfig.org/#download).
+Additionally, we use `deno fmt` to format the code in this repository.
+Using an autoformatter like this helps avoid (ongoing) disagreements over style preferences
+so we can focus on the content of the work.
+
+Even with these, it is possible that when you go to commit your changes,
+you have changed more than you intended to.
+One reason this can happen is that we might have forgetten to autoformat our code.
+
+One tool for this situation is `git add --patch` (or `-p`).
+This tool shows you each change in turn, allowing you to decide whether or not to
+"stage" the change. Stage only the chunks containing the pieces you meant to change,
+and commit those. You can then `git checkout .` to discard the remaining changes.
+
+It is sometimes unavoidable to reformat large portions of code.
+In these cases, please make a style-only commit,
+followed by a commit with the meaningful changes.
+This will speed up review significantly.
+
+Overall, a style-only pull request should be a rare event,
+typically when adopting or upgrading an autoformatter.
+This will generally have the effect of introducing merge conflicts,
+so we may refrain from doing it for a while.
+If you're feeling the urge, it's best to ask!
+
+
 [link_git]: https://git-scm.com/
 [link_handbook]: https://guides.github.com/introduction/git-handbook/
 [link_swc_intro]: http://swcarpentry.github.io/git-novice/
 [link_signupinstructions]: https://help.github.com/articles/signing-up-for-a-new-github-account
 [link_pullrequest]: https://help.github.com/articles/creating-a-pull-request-from-a-fork
 [link_fork]: https://help.github.com/articles/fork-a-repo/
 [link_clone]: https://help.github.com/articles/cloning-a-repository
+[conventional commits]: https://www.conventionalcommits.org/en/v1.0.0/
+[git blame]: https://docs.github.com/en/repositories/working-with-files/using-files/viewing-a-file#viewing-the-line-by-line-revision-history-for-a-file
+[scriv]: https://scriv.readthedocs.io/en/latest/
+[uvx]: https://docs.astral.sh/uv/guides/tools/

.github/workflows/web_build.yml

@@ -61,6 +61,13 @@ jobs:
         working-directory: dev/web
       - name: Nest dev inside stable
         run: mv dev/web/dist stable/web/dist/dev
+      - name: Add legacy/ redirect
+        run: |
+          mkdir $DIR
+          echo '<html><meta http-equiv="refresh" content="0; URL='$URL'"></html>' > $DIR/index.html
+        env:
+          DIR: stable/web/dist/legacy
+          URL: https://bids-standard.github.io/legacy-validator/
       - name: Upload GitHub Pages artifact
         uses: actions/upload-pages-artifact@v3
         with:

changelog.d/20241114_165444_effigies.md

@@ -0,0 +1,42 @@
+<!--
+A new scriv changelog fragment.
+
+Uncomment the section that is right (remove the HTML comment wrapper).
+-->
+
+<!--
+### Removed
+
+- A bullet item for the Removed category.
+
+-->
+<!--
+### Added
+
+- A bullet item for the Added category.
+
+-->
+<!--
+### Changed
+
+- A bullet item for the Changed category.
+
+-->
+<!--
+### Deprecated
+
+- A bullet item for the Deprecated category.
+
+-->
+### Fixed
+
+- Improve handling of `.bidsignore` files in the web validator.
+  Ignores matching directories but not the files they contained could fail to match.
+  (#113)
+
+<!--
+### Security
+
+- A bullet item for the Security category.
+
+-->

changelog.d/20241114_192344_effigies_scriv.md

@@ -0,0 +1,46 @@
+<!--
+A new scriv changelog fragment.
+
+Uncomment the section that is right (remove the HTML comment wrapper).
+-->
+
+<!--
+### Added
+
+- A bullet item for the Added category.
+
+-->
+<!--
+### Changed
+
+- A bullet item for the Changed category.
+
+-->
+<!--
+### Fixed
+
+- A bullet item for the Fixed category.
+
+-->
+<!--
+### Deprecated
+
+- A bullet item for the Deprecated category.
+
+-->
+<!--
+### Removed
+
+- A bullet item for the Removed category.
+
+-->
+<!--
+### Security
+
+- A bullet item for the Security category.
+
+-->
+### Infrastructure
+
+- Adopting [scriv](https://scriv.readthedocs.io/en/latest/) for changelog
+  management.

changelog.d/20241204_103403_effigies_nifti_headers.md

@@ -0,0 +1,49 @@
+<!--
+A new scriv changelog fragment.
+
+Uncomment the section that is right (remove the HTML comment wrapper).
+-->
+
+<!--
+### Added
+
+- A bullet item for the Added category.
+
+-->
+<!--
+### Changed
+
+- A bullet item for the Changed category.
+
+-->
+### Fixed
+
+- Resolve issue with parsing headers of NIfTI files with large extensions.
+  Fixes [issue 126].
+
+[issue 126]: https://github.com/bids-standard/bids-validator/issues/126
+
+<!--
+### Deprecated
+
+- A bullet item for the Deprecated category.
+
+-->
+<!--
+### Removed
+
+- A bullet item for the Removed category.
+
+-->
+<!--
+### Security
+
+- A bullet item for the Security category.
+
+-->
+<!--
+### Infrastructure
+
+- A bullet item for the Infrastructure category.
+
+-->

changelog.d/scriv.ini

@@ -0,0 +1,5 @@
+[scriv]
+format = md
+main_branches = master, main, dev
+version = literal: deno.json: version
+categories = Added, Changed, Fixed, Deprecated, Removed, Security, Infrastructure

src/files/browser.ts

@@ -52,7 +52,7 @@ export class BIDSFileBrowser implements BIDSFile {
 export async function fileListToTree(files: File[]): Promise<FileTree> {
   const ignore = new FileIgnoreRules([])
   const root = new FileTree('/', '/', undefined)
-  const tree = filesToTree(files.map((f) => new BIDSFileBrowser(f, ignore, root)))
+  const tree = filesToTree(files.map((f) => new BIDSFileBrowser(f, ignore, root)), ignore)
   const bidsignore = tree.get('.bidsignore')
   if (bidsignore) {
     try {

src/files/deno.ts

@@ -93,14 +93,17 @@ export class BIDSFileDeno implements BIDSFile {
 
   /**
    * Read bytes in a range efficiently from a given file
+   *
+   * Reads up to size bytes, starting at offset.
+   * If EOF is encountered, the resulting array may be smaller.
    */
   async readBytes(size: number, offset = 0): Promise<Uint8Array> {
     const handle = this.#openHandle()
     const buf = new Uint8Array(size)
     await handle.seek(offset, Deno.SeekMode.Start)
-    await handle.read(buf)
+    const nbytes = await handle.read(buf) ?? 0
     handle.close()
-    return buf
+    return buf.subarray(0, nbytes)
   }
 
   /**
@@ -151,17 +154,14 @@ async function _readFileTree(
  */
 export async function readFileTree(rootPath: string): Promise<FileTree> {
   const ignore = new FileIgnoreRules([])
-  try {
-    const ignoreFile = new BIDSFileDeno(
-      rootPath,
-      '.bidsignore',
-      ignore,
-    )
-    ignore.add(await readBidsIgnore(ignoreFile))
-  } catch (err) {
-    if (err && typeof err === 'object' && !('code' in err && err.code === 'ENOENT')) {
-      logger.error(`Failed to read '.bidsignore' file with the following error:\n${err}`)
+  const tree = await _readFileTree(rootPath, '/', ignore)
+  const bidsignore = tree.get('.bidsignore')
+  if (bidsignore) {
+    try {
+      ignore.add(await readBidsIgnore(bidsignore as BIDSFile))
+    } catch (err) {
+      console.log(`Failed to read '.bidsignore' file with the following error:\n${err}`)
     }
   }
-  return _readFileTree(rootPath, '/', ignore)
+  return tree
 }

src/files/filetree.test.ts

@@ -1,6 +1,6 @@
 import { assert, assertEquals } from '@std/assert'
 import type { FileIgnoreRules } from './ignore.ts'
-import type { FileTree } from '../types/filetree.ts'
+import type { BIDSFile, FileTree } from '../types/filetree.ts'
 import { filesToTree, pathsToTree } from './filetree.ts'
 
 Deno.test('FileTree generation', async (t) => {
@@ -50,4 +50,19 @@ Deno.test('FileTree generation', async (t) => {
     assert(tree.contains(['sub-01', 'anat']))
     assert(tree.contains(['sub-01', 'anat', 'sub-01_T1w.nii.gz']))
   })
+  await t.step('converts a list with ignores', async () => {
+    const tree = pathsToTree(
+      ['/dataset_description.json', '/README.md', '/.bidsignore', '/bad_file'],
+      ['bad_file'],
+    )
+    assertEquals(tree.directories, [])
+    assertEquals(tree.files.map((f) => f.name), [
+      'dataset_description.json',
+      'README.md',
+      '.bidsignore',
+      'bad_file',
+    ])
+    const bad_file = tree.get('bad_file') as BIDSFile
+    assert(bad_file.ignored)
+  })
 })

src/files/filetree.ts

@@ -1,27 +1,29 @@
 import { parse, SEPARATOR_PATTERN } from '@std/path'
 import * as posix from '@std/path/posix'
 import { type BIDSFile, FileTree } from '../types/filetree.ts'
+import { FileIgnoreRules } from './ignore.ts'
 
 const nullFile = {
   size: 0,
-  ignored: false,
   stream: new ReadableStream(),
   text: () => Promise.resolve(''),
   readBytes: async (size: number, offset?: number) => new Uint8Array(),
   parent: new FileTree('', '/'),
   viewed: false,
 }
 
-export function pathToFile(path: string): BIDSFile {
+export function pathToFile(path: string, ignored: boolean = false): BIDSFile {
   const name = path.split('/').pop() as string
-  return { name, path, ...nullFile }
+  return { name, path, ignored, ...nullFile }
 }
 
-export function pathsToTree(paths: string[]): FileTree {
-  return filesToTree(paths.map(pathToFile))
+export function pathsToTree(paths: string[], ignore?: string[]): FileTree {
+  const ignoreRules = new FileIgnoreRules(ignore ?? [])
+  return filesToTree(paths.map((path) => pathToFile(path, ignoreRules.test(path))))
 }
 
-export function filesToTree(fileList: BIDSFile[]): FileTree {
+export function filesToTree(fileList: BIDSFile[], ignore?: FileIgnoreRules): FileTree {
+  ignore = ignore ?? new FileIgnoreRules([])
   const tree: FileTree = new FileTree('/', '/')
   for (const file of fileList) {
     const parts = parse(file.path)
@@ -37,7 +39,7 @@ export function filesToTree(fileList: BIDSFile[]): FileTree {
         current = exists
         continue
       }
-      const newTree = new FileTree(posix.join(current.path, level), level, current)
+      const newTree = new FileTree(posix.join(current.path, level), level, current, ignore)
       current.directories.push(newTree)
       current = newTree
     }