Adding the last line of a multi-line diagnostics.

[Tomer-StarkWare]

No description provided.

[reviewable-StarkWare]

This change is 

[orizi]

Reviewed all commit messages.
Reviewable status: 0 of 41 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware and @Tomer-StarkWare)

---

crates/cairo-lang-diagnostics/src/diagnostics_test.rs line 66 at r1 (raw file):

                 dummy_file.sierra:2:2
            efg.
            ***^

or something like that?
cause this one seems unclear.

Suggestion:

             --> dummy_file.sierra:1:1-2:2
            V***
            abcd
            efg.
            ***^

[Tomer-StarkWare]

Reviewable status: 0 of 41 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware and @orizi)

---

crates/cairo-lang-diagnostics/src/diagnostics_test.rs line 66 at r1 (raw file):

Previously, orizi wrote…

or something like that?
cause this one seems unclear.

Done.

[Tomer-StarkWare]

Reviewable status: 0 of 130 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware and @orizi)

---

crates/cairo-lang-diagnostics/src/diagnostics_test.rs line 66 at r1 (raw file):

Previously, Tomer-StarkWare (TomerC-StarkWare) wrote…

Done.

How about now?

[orizi]

Reviewed 5 of 142 files at r4.
Reviewable status: 5 of 142 files reviewed, 3 unresolved discussions (waiting on @gilbens-starkware and @Tomer-StarkWare)

---

crates/cairo-lang-diagnostics/src/location_marks.rs line 9 at r4 (raw file):

mod test;

pub fn get_location_marks(

this one is now only for single file?
if so - rename and doc.

Code quote:

pub fn get_location_marks(

---

crates/cairo-lang-diagnostics/src/location_marks.rs line 44 at r4 (raw file):

            res.push('^');
        }
    }

Suggestion:

    

    let subspan_in_first_line =
        TextSpan { start: span.start, end: std::cmp::min(first_line_end, span.end) };
    let marker_length = subspan_in_first_line.n_chars(&content);
    res.extend(repeat_n('^', marker_length));

---

crates/cairo-lang-diagnostics/src/location_marks.rs line 49 at r4 (raw file):

}

pub fn get_multiple_lines_location_marks(

doc.

add boolean to skip printing middle lines.
(have | ... or something instead.

[Tomer-StarkWare]

Reviewable status: 2 of 146 files reviewed, 3 unresolved discussions (waiting on @gilbens-starkware and @orizi)

---

crates/cairo-lang-diagnostics/src/location_marks.rs line 9 at r4 (raw file):

Previously, orizi wrote…

this one is now only for single file?
if so - rename and doc.

Done.

---

crates/cairo-lang-diagnostics/src/location_marks.rs line 49 at r4 (raw file):

Previously, orizi wrote…

doc.

add boolean to skip printing middle lines.
(have | ... or something instead.

Done.

---

crates/cairo-lang-diagnostics/src/location_marks.rs line 44 at r4 (raw file):

            res.push('^');
        }
    }

Done.

[orizi]

Reviewed 1 of 23 files at r5, all commit messages.
Reviewable status: 3 of 146 files reviewed, 4 unresolved discussions (waiting on @gilbens-starkware and @Tomer-StarkWare)

---

crates/cairo-lang-diagnostics/src/diagnostics.rs line 134 at r5 (raw file):

                            db,
                            &user_location,
                            starting_text_pos.line + 3 < ending_text_pos.line,

comment as to why.
(just creating 3 as a const with a good name above the marks = can be enough)

[orizi]

Reviewed 1 of 23 files at r5.
Reviewable status: 4 of 146 files reviewed, 5 unresolved discussions (waiting on @gilbens-starkware and @Tomer-StarkWare)

---

crates/cairo-lang-diagnostics/src/location_marks.rs line 41 at r5 (raw file):

        TextSpan { start: span.start, end: std::cmp::min(first_line_end, span.end) };
    let marker_length = subspan_in_first_line.n_chars(&content);
    println!("marker_length: {}", marker_length);

remove.

Code quote:

    println!("marker_length: {}", marker_length);

---

crates/cairo-lang-diagnostics/src/location_marks.rs line 43 at r5 (raw file):

    println!("marker_length: {}", marker_length);
    // marker_length can be 0 if the span is empty.
    res.extend(repeat_n('^', max(marker_length, 1)));

since just single use - consider:

Suggestion:

    res.extend(repeat_n('^', std::cmp::max(marker_length, 1)));

---

crates/cairo-lang-diagnostics/src/location_marks.rs line 77 at r5 (raw file):

    } else {
        res += "| ...\n";
    }

put the 3 constant here.
from the outside you should just decide if you want this concept.

Code quote:

    if !skip_middle_lines {
        for row_index in first_line_idx + 1..=last_line_idx - 1 {
            res += &get_line_content(summary.clone(), row_index, content.clone(), false);
        }
    } else {
        res += "| ...\n";
    }

---

crates/cairo-lang-diagnostics/src/location_marks.rs line 113 at r5 (raw file):

    };
    res += line_span.take(&content);
    res.push('\n');

Suggestion:

    format!("{}{content}\n", if first_line { " " } else { "| " })

[Tomer-StarkWare]

Reviewable status: 4 of 146 files reviewed, 5 unresolved discussions (waiting on @gilbens-starkware and @orizi)

---

crates/cairo-lang-diagnostics/src/diagnostics.rs line 134 at r5 (raw file):

Previously, orizi wrote…

comment as to why.
(just creating 3 as a const with a good name above the marks = can be enough)

Done.

---

crates/cairo-lang-diagnostics/src/location_marks.rs line 41 at r5 (raw file):

Previously, orizi wrote…

remove.

Done.

---

crates/cairo-lang-diagnostics/src/location_marks.rs line 43 at r5 (raw file):

Previously, orizi wrote…

since just single use - consider:

Done.

---

crates/cairo-lang-diagnostics/src/location_marks.rs line 77 at r5 (raw file):

Previously, orizi wrote…

put the 3 constant here.
from the outside you should just decide if you want this concept.

Not sure I understand. There is now a boolean that the function gets (not related to the number of lines), and a check in the function regarding the number of lines

---

crates/cairo-lang-diagnostics/src/location_marks.rs line 113 at r5 (raw file):

    };
    res += line_span.take(&content);
    res.push('\n');

Done.

[orizi]

Reviewed 1 of 2 files at r6, all commit messages.
Reviewable status: 4 of 146 files reviewed, 2 unresolved discussions (waiting on @gilbens-starkware and @Tomer-StarkWare)

---

crates/cairo-lang-diagnostics/src/location_marks.rs line 77 at r5 (raw file):

Previously, Tomer-StarkWare (TomerC-StarkWare) wrote…

Not sure I understand. There is now a boolean that the function gets (not related to the number of lines), and a check in the function regarding the number of lines

something like what you did.

---

crates/cairo-lang-diagnostics/src/location_marks.rs line 69 at r6 (raw file):

        span.end.position_in_file(db, location.file_id).expect("Failed to find location in file.");

    let minimum_lines_to_show = 3;

Suggestion:

    CONST LINES_TO_REPLACE_MIDDLE: usize = 4;

[Tomer-StarkWare]

Reviewable status: 4 of 146 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware and @orizi)

---

crates/cairo-lang-diagnostics/src/location_marks.rs line 69 at r6 (raw file):

        span.end.position_in_file(db, location.file_id).expect("Failed to find location in file.");

    let minimum_lines_to_show = 3;

Done.

[orizi]

Reviewed all commit messages.
Reviewable status: 4 of 146 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware and @Tomer-StarkWare)

---

crates/cairo-lang-diagnostics/src/location_marks.rs line 70 at r7 (raw file):

    const LINES_TO_REPLACE_MIDDLE: usize = 4;
    if !skip_middle_lines || first_line_idx + LINES_TO_REPLACE_MIDDLE >= last_line_idx {

Suggestion:

if !skip_middle_lines || first_line_idx + LINES_TO_REPLACE_MIDDLE > last_line_idx {

[orizi]

Reviewed 2 of 142 files at r4, 1 of 23 files at r5, all commit messages.
Reviewable status: 7 of 146 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware and @Tomer-StarkWare)

[Tomer-StarkWare]

Reviewable status: 7 of 146 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware and @orizi)

---

crates/cairo-lang-diagnostics/src/location_marks.rs line 70 at r7 (raw file):

    const LINES_TO_REPLACE_MIDDLE: usize = 4;
    if !skip_middle_lines || first_line_idx + LINES_TO_REPLACE_MIDDLE >= last_line_idx {

Done.

[orizi]

Reviewed 1 of 142 files at r4, 1 of 23 files at r5, 2 of 6 files at r9, all commit messages.
Reviewable status: 11 of 146 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware and @Tomer-StarkWare)

---

crates/cairo-lang-diagnostics/src/location_marks.rs line 1 at r9 (raw file):

use std::sync::Arc;

lets make get_single and get_multiple non-pub.
and have an external caller using the more relevant one.

[Tomer-StarkWare]

Reviewable status: 6 of 146 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware and @orizi)

---

crates/cairo-lang-diagnostics/src/location_marks.rs line 1 at r9 (raw file):

Previously, orizi wrote…

lets make get_single and get_multiple non-pub.
and have an external caller using the more relevant one.

Done.

[orizi]

Reviewed 15 of 142 files at r4, 1 of 6 files at r9, 5 of 7 files at r10, all commit messages.
Reviewable status: 27 of 146 files reviewed, all discussions resolved (waiting on @gilbens-starkware)

[orizi]

Reviewed 103 of 142 files at r4, 11 of 23 files at r5, 3 of 6 files at r9, 2 of 7 files at r10.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @gilbens-starkware)

[orizi]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @gilbens-starkware)

[orizi]

Reviewed 35 of 50 files at r11, all commit messages.
Reviewable status: 131 of 146 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware and @Tomer-StarkWare)

---

crates/cairo-lang-sierra-generator/src/statement_location_test_data/simple line 1149 at r11 (raw file):

|         MyEnum::A((_, x)) | MyEnum::B((x, _)) => x,
|         MyEnum::C((x, _, t)) | MyEnum::D(P { x, y: _, z: t }) => (x + t).into(),
|     }

should it be?

Suggestion:

 _____^
| ...
|     }

[orizi]

Reviewed 15 of 50 files at r11.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware and @Tomer-StarkWare)

[Tomer-StarkWare]

Reviewable status: 135 of 146 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware and @orizi)

---

crates/cairo-lang-sierra-generator/src/statement_location_test_data/simple line 1149 at r11 (raw file):

Previously, orizi wrote…

should it be?

Yes it should, fixed

[orizi]

Reviewed 11 of 11 files at r12, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @gilbens-starkware)

[orizi]

Reviewed all commit messages.
Reviewable status: 145 of 146 files reviewed, all discussions resolved (waiting on @gilbens-starkware)