Skip to content

Don't display frame ID on stack trace - #933

Open
samisalreadytaken wants to merge 1 commit into
puremourning:masterfrom
samisalreadytaken:stacktrace
Open

samisalreadytaken wants to merge 1 commit into
puremourning:masterfrom
samisalreadytaken:stacktrace

Conversation

@samisalreadytaken

Copy link
Copy Markdown
Contributor

Frame ID is an opaque data that does not concern the end user. Different servers assign them differently. For example this python server has IDs 3,2,12,24; this c server has [1000,1007]; and my debugger simply increments an index with no order.

Don't print this internal info to save screen space and prevent potential confusion.

@puremourning

Copy link
Copy Markdown
Owner

This change is Reviewable

@puremourning puremourning left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Good idea.

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @samisalreadytaken)


python3/vimspector/stack_trace.py line 684 at r1 (raw file):

          self._buf,
          '  {0}@{1}:{2}'.format( frame[ 'name' ],
                                       source[ 'name' ],

Fix formatting.

@puremourning

Copy link
Copy Markdown
Owner

Thanks! Just a nit and we can merge

@samisalreadytaken

Copy link
Copy Markdown
Contributor Author

Should the lines be indented for clarity?

- Thread 0: T#0 (paused)
  f2()@script.nut:10
  f1()@script.nut:20
  main()@script.nut:30
+ Thread 1: T#1 (paused)
+ Thread 2: T#2 (paused)
- Thread 0: T#0 (paused)
    f2()@script.nut:10
    f1()@script.nut:20
    main()@script.nut:30
+ Thread 1: T#1 (paused)
+ Thread 2: T#2 (paused)

@puremourning

Copy link
Copy Markdown
Owner

Should the lines be indented for clarity?

- Thread 0: T#0 (paused)
  f2()@script.nut:10
  f1()@script.nut:20
  main()@script.nut:30
+ Thread 1: T#1 (paused)
+ Thread 2: T#2 (paused)
- Thread 0: T#0 (paused)
   f2()@script.nut:10
   f1()@script.nut:20
   main()@script.nut:30
+ Thread 1: T#1 (paused)
+ Thread 2: T#2 (paused)

Hmm. Yeah maybe. The old id served as a visual marker but now we have a little ambiguity. Try it. See what you think.

@samisalreadytaken

Copy link
Copy Markdown
Contributor Author

I think it's better with it, also more consistent with the "tree" style expand.

- Thread 0: MainThread (paused)
    main()@script.nut:36
- Thread 1: Thread 0x000002866CFFBEA0 (paused)
    ft2()@script.nut:15
    ft1()@script.nut:20
    ft()@script.nut:26
- Thread 2: Thread 0x000002866D00FEA0 (paused)
    ft2()@script.nut:15
    ft1()@script.nut:20
    ft()@script.nut:26
- Thread 0: MainThread (paused)
  main()@script.nut:36
- Thread 1: Thread 0x000002866CFFBEA0 (paused)
  ft2()@script.nut:15
  ft1()@script.nut:20
  ft()@script.nut:26
- Thread 2: Thread 0x000002866D00FEA0 (paused)
  ft2()@script.nut:15
  ft1()@script.nut:20
  ft()@script.nut:26

@puremourning

Copy link
Copy Markdown
Owner

Agreed

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 this pull request may close these issues.

2 participants