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

Bug in how the title highlighter handles document titles with no spaces #236

Closed
aFuzzyBear opened this issue Jan 13, 2022 · 10 comments · Fixed by #243
Closed

Bug in how the title highlighter handles document titles with no spaces #236

aFuzzyBear opened this issue Jan 13, 2022 · 10 comments · Fixed by #243
Assignees

Comments

@aFuzzyBear
Copy link

Hi Jamie,

Sorry for not getting back to your message, at the time I was that distant from this project I had nothing of note to add. However I did decide to give your wee project another shot, since the UI Framework I was seeking to implement this with has became more mature and stable,

I managed to get Strok installed with little effort, my first cargo package 🥳
I must ask if there is a way for this to get punted into the node_modules/bin/ directory this way it would be as simple to launch using yarn stork ... Im unsure how this would be setup with any CI/CD, using cargo etc im not sure how that jigsaw looks from here,

The bug I wish to report was with the searching part of the program, it seems to be panicking and not returning any results, in dev tools I have copied the entire stack errors for you, *my apologies,

I have linked to the repo where this is being ran on, Stork Branch

Its simple to spin up, the working directory is www/XElement and yarn && yarn start would get you going, if you then do your

strok test --input src/config/StorkFileIndex.toml

You should see all the jazzy funkyness that is going on, on the localhost:1612

I appreciate any assistance that you might have towards this, I would love to see this implemented on the fledgling astro-ui project that Ive got going, also would like to see this project work close with other astro projects if we can get a few things sorted out, if you dont mind I would be limited in my contributions since Im not at all proficient in anything really let alone rust 😅, but I am helpful if there is anything I can do to reciprocate in kind,

Kindest Thanks

Peter

@jameslittle230
Copy link
Owner

jameslittle230 commented Jan 14, 2022

@aFuzzyBear,

I'm having trouble understanding what you're saying -- let me try to summarize (and please let me know if I've misinterpreted or left something out):

  1. You're curious where Cargo is putting the binary after building the Stork crate so you can move it into a different directory and eventually use it in a NodeJS environment
  2. You're seeing a WASM bug, and the logs you're seeing have been uploaded to a branch of one of your Github repositories here

For 1, Cargo will put the binary in a directory called target by default. On my system, after running cargo build --release, the output binary will be located at target/release/stork. This might be different based on your system and configuration. I also have an open issue (#129) for using NPM to install Stork, if you're interested in watching that.

For 2, the most helpful error message I see is

panicked at 'range end index 2 out of range for slice of length 1', src/index_versions/v3/search/entry_and_intermediate_excerpts.rs:144:33

That line of code (with context) is:

.map(|ie| {
let space_offset = if ie.word_index == 0 { 0 } else { 1 };
let beginning = split_title[0..ie.word_index].join(" ").len() + space_offset;
HighlightRange {
beginning,
end: beginning + ie.query.len(),
}
})

which looks like an error trying to segment a document's title into highlighted and non-highlighted substrings.

I notice that none of your titles are strings with spaces in them, and looking at the code I think I see how that might reveal a bug. I'll try to have this fixed in the next release, 1.4.0, coming soon.


Two final points:

  1. My name is James, I'd prefer that you not call me Jamie.
  2. Your message makes it sound like we've worked together before, but I can't find any prior conversations I've had with you. Could you remind me where we've chatted before?

Thanks!

@jameslittle230 jameslittle230 changed the title BUG: HAving issues with the Searching part of Stork Bug in how the title highlighter handles document titles with no spaces Jan 14, 2022
@aFuzzyBear
Copy link
Author

Ah sorry James, my mistake, wont happen again,
Thank you for getting back to me so quickly, with regards to the issue about the runtime panic, I can appreciate that this might raise a bug, I can try to format the string better on my end and see if that makes any difference.

In regards to the first issue, you are correct, I am curious to see how this plays out, in terms of deployment etc, I only am familiar with node and its runtime only briefly, I would be interested in following the developments of the pr#129

In regards to the issue of us 'conversing' prior, there was an issue raised ages ago, you did respond however I didnt for that I was apologising for.

@aFuzzyBear
Copy link
Author

I was wondering if my index was being compiled correctly, It appears as so it is, however if there is something else that might be tripping it up on my end, please do let me know,
My Thanks again,

@jameslittle230
Copy link
Owner

Nope, I think you're using Stork totally correctly, this is just a bug in the highlighter!

Thanks for submitting the bug report, I appreciate it :)

James

@aFuzzyBear
Copy link
Author

Thank you for your efforts and labour, I really like this wee project, it must be the picture of the stork, idk.
Please do let me know when version 1.4 is out, looking forward to it =)

@jameslittle230
Copy link
Owner

Note to self: was able to repro the panic by using the attached index and searching for animate. The IntermediateExcerpt holds a word_index of 2.

@jameslittle230
Copy link
Owner

Ah, it's because when the indexer is splitting a word, it's splitting on whitespace and hyphens, whereas the split_title is only splitting on whitespace.

@aFuzzyBear
Copy link
Author

Hi James, I notice you closed the PR, does this mean that this wee problem has been fixed? If so would I need to do anything to apply it or do anything different?
Also thank you for getting this fix together, I really appreciate it

@jameslittle230
Copy link
Owner

The bug is fixed in the master branch, and will be released in 1.4.0 later today.

@aFuzzyBear
Copy link
Author

excellent, thank you for this, I really appreciate it, Ill post back with results, and hopefully be able to demonstrate it working =)

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 a pull request may close this issue.

2 participants