Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Syntactic Indexing: add TAR archive indexing mode to scip-syntax CLI#63097

Merged
keynmol merged 23 commits into
mainfrom
scip-syntax-cli-tar-mode
Jun 6, 2024
Merged

Syntactic Indexing: add TAR archive indexing mode to scip-syntax CLI#63097
keynmol merged 23 commits into
mainfrom
scip-syntax-cli-tar-mode

Conversation

@keynmol

@keynmol keynmol commented Jun 5, 2024

Copy link
Copy Markdown
Contributor

Fixes GRAPH-651
Fixes GRAPH-650

New features:

  • index tar <input> allows indexing tar archives when input is a file, and stdin when input is literal string -
  • BREAKING: Instead of flags controlling the type of input source, we now have subcommands: index files, index workspace, index tar

Refactoring:

  • Tests were improved, with helper functions broken down into composable pieces, and producing 1 snapshot per test, making it easier to manage
  • Closures were removed from the indexing code, instead replaced with functions.
  • Calls to .unwrap were replaced with better error handling code
  • Path canonicalisation was replaced with path cleanup + absolutisation - to avoid following symlinks (which is what canonicalize would do)

Most of the refactoring was triggered by the changes required to add more tests.

Test plan

  • New integration tests

@cla-bot cla-bot Bot added the cla-signed label Jun 5, 2024
@github-actions github-actions Bot added team/graph Graph Team (previously Code Intel/Language Tools/Language Platform) team/product-platform labels Jun 5, 2024
@keynmol keynmol force-pushed the scip-syntax-cli-tar-mode branch from c2f2cff to cbbd175 Compare June 5, 2024 15:56
@keynmol keynmol force-pushed the scip-syntax-cli-tar-mode branch from b976a70 to 1188f9d Compare June 6, 2024 12:06
@keynmol keynmol changed the title Scip syntax cli tar mode Syntactic Indexing: add TAR archive indexing mode to scip-syntax CLI Jun 6, 2024
keynmol added 4 commits June 6, 2024 14:10
⠀⠀⠀⠀⠀⠀⠀⠀⢀⣀⣤⣤⣄⣀⠀⠀⠀⠀⠀⠀⠀
⠀⠀⠀⠀⠀⠀⢠⣶⠟⣉⣤⣢⣄⡪⢝⢦⡀⠀⠀⠀⠀
⠀⠀⠀⠀⠀⢰⡿⢁⣾⠟⠉⠉⠉⠹⣧⣃⢳⡀⠀⠀⠀
⠀⠀⠀⢀⣀⣼⡏⣼⠃⠀⠀⠀⠀⠀⢹⣏⣸⡅⠀⠀⠀
⠀⢀⣴⡿⠿⣿⠃⣿⠀⠀⠀⠀⠀⠀⣸⣷⣿⣶⣄⠀⠀
⠠⠞⠁⠀⢠⣿⠌⣿⠀⠀⠀⠀⠀⠀⣿⡇⣿⠛⠛⠿⣄
⠀⠀⢀⣠⠾⠿⠾⣷⡀⠀⠀⠀⡠⢶⠛⠹⠿⢶⣄⠀⠈
⠀⢠⠋⠀⢀⣁⡀⠘⠙⣦⡀⠘⠈⠀⣠⣤⡀⠀⠻⣦⠀
⠀⢀⠀⠀⢾⣿⣿⠀⠀⢘⣧⠇⡀⠘⢿⣿⠏⠀⠀⡿⠀
⠀⠈⢧⡀⠈⣉⡁⠀⣤⡞⠀⠘⢢⣀⡄⠀⢠⣠⠾⠃⠀
⠀⠀⠀⠉⣷⡖⣶⡛⠉⠀⠀⠀⠀⣿⡏⣿⠋⠁⠀⠀⠀
⠀⠀⠀⠀⢻⡇⣽⢺⣱⡄⠀⠀⠀⣿⢇⡏⠀⠀⣰⡖⣦
⠀⠀⠀⠀⣿⡇⣿⢻⠸⡇⠀⠀⠀⣿⢰⡏⢀⣾⢳⡾⠉
⠀⠀⠀⠀⣿⡄⡿⣿⠘⡁⠀⠀⠐⣿⢸⡇⣾⢇⡿⠀⠀
⠀⠀⠀⠀⣿⠐⣟⣧⢰⠀⠀⠀⢸⣿⢺⠆⣿⢸⡇⠀⠀
⠀⠀⠀⠀⣿⠡⣟⣿⢸⡇⠀⠀⢸⣇⢿⠆⣿⢸⡅⠀⠀
⠀⠀⠀⠀⣿⠡⣏⣿⡸⡅⠀⠀⣼⢏⣼⠆⣿⢸⠃⠀⠀
⠀⠀⠀⠀⣿⠰⣿⠹⣶⣭⣖⣪⣵⡾⠏⢠⣿⢸⡁⠀⠀
⠀⠀⠀⠀⣿⢂⡷⠀⠈⠉⠘⠉⠉⠀⠀⠸⣿⢼⡀⠀⠀
⠀⠀⠀⠀⣿⡍⢿⡀⠀⠀⠀⠀⠀⠀⠀⣸⠇⣼⠀⠀⠀
⠀⠀⠀⠀⠹⣯⡎⡻⢦⣀⣀⣀⣀⡤⠞⣉⣼⠃⠀⠀⠀
⠀⠀⠀⠀⠀⠈⠻⢷⣦⣢⣬⣤⣤⣶⠾⠋⠀⠀⠀⠀⠀
⠀⠀⠀⠀⠀⠀⠀⠀⠀⠉⠉⠉⠀⠀⠀⠀⠀⠀⠀⠀⠀
@keynmol keynmol marked this pull request as ready for review June 6, 2024 13:37
@keynmol keynmol requested a review from kritzcreek June 6, 2024 13:38
tags = [TAG_PLATFORM_GRAPH],
deps = all_crate_deps(
normal = True,
normal_dev = True,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this allows bringing in dev-dependencies from Cargo

@kritzcreek kritzcreek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks really nice. I'll take another look tomorrow

Comment thread docker-images/syntax-highlighter/crates/scip-syntax/src/main.rs Outdated
Comment thread docker-images/syntax-highlighter/crates/scip-syntax/tests/integration_test.rs Outdated
Comment thread docker-images/syntax-highlighter/crates/scip-syntax/src/main.rs
Comment thread docker-images/syntax-highlighter/crates/scip-syntax/src/main.rs Outdated

@kritzcreek kritzcreek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't want to block the merge after updating the error message for --workspace

LGTM

@keynmol keynmol merged commit dfa60d6 into main Jun 6, 2024
@keynmol keynmol deleted the scip-syntax-cli-tar-mode branch June 6, 2024 18:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed team/graph Graph Team (previously Code Intel/Language Tools/Language Platform) team/product-platform

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants