[PATCH] D11877: [Support] On Windows, generate PDF files for graphs and open with associated viewer

[Michael Kruse via llvm-commits]

Thanks for the feedback.

2015-08-17 18:16 GMT+02:00 Aaron Ballman <aaron at aaronballman.com>:
>> -  enum PSViewerKind { PSV_None, PSV_OSXOpen, PSV_XDGOpen, PSV_Ghostview };
>> -  PSViewerKind PSViewer = PSV_None;
>> +  enum ViewerKind {
>> +    PSV_None,
>> +    PSV_OSXOpen,
>> +    PSV_XDGOpen,
>> +    PSV_Ghostview,
>> +    PDFV_CmdStart
>
> Why is this prefixed with PDFV_ instead of PSV_ like the rest of the
> enumerators? Should have the same prefix since it's part of the same
> enumeration.

Because PS stands for "PostScript" while this will open a PDF viewer.

However, considering that I renamed the enum's name as well, I can
find a common prefix. ("V_" ?)


>> +#ifdef LLVM_ON_WIN32
>> +  if (!Viewer && S.TryFindProgram("cmd", ViewerPath)) {
>> +    Viewer = PDFV_CmdStart;
>> +  }
>
> Is there an advantage to using cmd instead of manually opening the
> file with the related PDF viewer via ShellExecuteEx (or one of the
> other Shell APIs)?

The Support cross-platform layer doesn't expose such API (at least I
am not aware of). Since this function is only useful at debug-time,
calling cmd seems like a sufficient solution to me. I am not sure
whether including Windows.h into this file will find general
acceptance.

Michael