This is an archive of the discontinued LLVM Phabricator instance.

[analyzer] Highlight arrows for currently selected event
ClosedPublic

Authored by vsavchenko on Dec 9 2020, 3:17 AM.

Details

Summary

In some cases, when the execution path of the diagnostic
goes back and forth, arrows can overlap and create a mess.
Dimming arrows that are not relevant at the moment, solves this issue.
They are still visible, but don't draw too much attention.

Diff Detail

Event Timeline

vsavchenko created this revision.Dec 9 2020, 3:17 AM
vsavchenko requested review of this revision.Dec 9 2020, 3:17 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 9 2020, 3:17 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Here is the HTML file for the test.

HTML sample looks fine! But there is the same problem as in D92639. IE doesn't draw arrows.

clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
162
  1. Whether size() is not an aim?
  2. It can fail in case of empty container.
vsavchenko added inline comments.Feb 5 2021, 1:37 AM
clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
162

size() doesn't mean the same semantically. This structure is at least of size 1. Please, consider the following code on how this data structure is constructed and used.

I've made some debugging with IE. Let's keep the diagnostics IE friendly. We might not know how many users work with IE in real.

clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
1292–1300

IE doesn't support classList for elements.

1335–1337

Let's make this more sofisticated, as it only works for English keyboard layout with Caps off.

1352–1355

IE doesn't support Array.from/fill functions.

1374
ASDenysPetrov added a comment.EditedJul 20 2021, 7:23 AM

P.S. Sorry. My prev comments get dimmed, since I've been playing with history commits, but they still are relevant.

Maybe I'm missing something, but do we really need to care about IE? The last version was released in 2013, and even Microsoft itself stops supporting IE. Why should we care? Is there anyone who uses old deprecated browser that is not maintained? classList thing was here for almost 4 years and no one seemed to care. Am I missing something here?

I want to say that I really appreciate the effort you put into finding all the workarounds for IE, but it makes adding new features here incredibly painful because IE doesn't seem to support anything. And the majority of developers (on Linux and on MacOS) have literally no way to test it. What we gain from supporting IE for non-existing users, we lose in the ability to actually improve this code!

ASDenysPetrov added a comment.EditedJul 20 2021, 8:16 AM

IE doesn't seem to support anything. And the majority of developers (on Linux and on MacOS) have literally no way to test it. What we gain from supporting IE for non-existing users, we lose in the ability to actually improve this code!

Oh, I absolutely agree with every of your statements. I'm not really excited to test and adjust this stuff for IE. But I occasionally get requests from users which use IE and, get ready, the internal browser of Visual Studio :D which exploits IE core. So I kindly ask you to keep the JS IE friendly at least for a while. We don't really make changes too often here.

Or, we can find another symbiotic way. You can make changes the way without painfull part of thinking about IE. And I will prepare the next patch adjusting it. Thus, revisions would be smaller. That's would be easier for you to test all the things before the load. I will take a charge for IE part on my own and prepare a new revision.

ASDenysPetrov accepted this revision.Jul 29 2021, 7:06 AM

Let's load this patch and I will prepare the adjustment for IE in pursuit.

This revision is now accepted and ready to land.Jul 29 2021, 7:06 AM

Or, we can find another symbiotic way. You can make changes the way without painfull part of thinking about IE. And I will prepare the next patch adjusting it. Thus, revisions would be smaller. That's would be easier for you to test all the things before the load. I will take a charge for IE part on my own and prepare a new revision.

That actually can work! Thanks!

Let's load this patch and I will prepare the adjustment for IE in pursuit.

Ah, OK. Sure, let's do it!