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

fix: Always trim source context lines #1443

Merged
merged 3 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/symbolicator-js/src/symbolication.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ async fn symbolicate_js_frame(

fn apply_source_context(frame: &mut JsFrame, source: &str) -> Result<(), JsModuleErrorKind> {
let lineno = frame.lineno as usize;
let column = frame.colno.map(|col| col as usize);
let column = frame.colno.map(|col| col as usize).unwrap_or_default();

if let Some((pre_context, context_line, post_context)) =
get_context_lines(source, lineno, column, None)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ impl ModuleLookup {

pub(crate) fn set_source_context(source: &str, frame: &mut RawFrame) -> Option<()> {
let (pre_context, context_line, post_context) =
get_context_lines(source, frame.lineno?.try_into().ok()?, None, None)?;
get_context_lines(source, frame.lineno?.try_into().ok()?, 0, None)?;
frame.pre_context = pre_context;
frame.context_line = Some(context_line);
frame.post_context = post_context;
Expand Down
2 changes: 1 addition & 1 deletion crates/symbolicator-proguard/src/symbolication.rs
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ impl ProguardService {
};

if let Some((pre_context, context_line, post_context)) =
get_context_lines(contents, lineno as usize, None, None)
get_context_lines(contents, lineno as usize, 0, None)
{
frame.pre_context = pre_context;
frame.context_line = Some(context_line);
Expand Down
21 changes: 8 additions & 13 deletions crates/symbolicator-service/src/source_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,33 +4,28 @@ pub const DEFAULT_CONTEXT_LINES: usize = 5;
///
/// N (`context_lines`) lines of context will be resolved in addition to `lineno`.
/// This defaults to [`DEFAULT_CONTEXT_LINES`].
/// If `trim_to_column` is provided, every output line will be truncated to ~150 characters
/// centered around the provided column number, and a `{snip}` marker is added at the trimmed edges.
/// Every output line will be truncated to ~150 characters, centered as best as possible
loewenheim marked this conversation as resolved.
Show resolved Hide resolved
/// around `trim_to_column`, and a `{snip}` marker is added at the trimmed edges.
/// To simply cut off lines after ~150 characters, pass 0.
///
/// If no source line for `lineno` is found in `source`, it will return [`None`] instead.
pub fn get_context_lines(
source: &str,
lineno: usize,
trim_to_column: Option<usize>,
trim_to_column: usize,
context_lines: Option<usize>,
) -> Option<(Vec<String>, String, Vec<String>)> {
let context_lines = context_lines.unwrap_or(DEFAULT_CONTEXT_LINES);

let start_line = lineno.saturating_sub(context_lines).saturating_sub(1);
let line_diff = (lineno - start_line).saturating_sub(1);

let maybe_trim_line = |line: &str| {
if let Some(column) = trim_to_column {
trim_context_line(line, column)
} else {
line.to_string()
}
};
let trim_line = |line: &str| trim_context_line(line, trim_to_column);

let mut lines = source.lines().skip(start_line);
let pre_context = (&mut lines).take(line_diff).map(maybe_trim_line).collect();
let context = maybe_trim_line(lines.next()?);
let post_context = lines.take(context_lines).map(maybe_trim_line).collect();
let pre_context = (&mut lines).take(line_diff).map(trim_line).collect();
let context = trim_line(lines.next()?);
let post_context = lines.take(context_lines).map(trim_line).collect();

Some((pre_context, context, post_context))
}
Expand Down