fixed a couple things with get_directory()

Reporter: time-killer-games  |  Status: closed  |  Last Modified: March 27, 2019, 12:28:30 AM

  • Robert seemed to not know how to set the default directory, so I took care of that for him.
  • He wrote it originally with a bug regarding the flags which allowed selecting non-directory locations that returned CLSID's instead of paths, which I also fixed by preventing those from being selectable, forcing only valid paths to be selectable.
  • Fixed some things with Zenity and KDialog that Josh asked me to do in my previous pull request but he merged it before I could do so, (he said I didn't have to do it, but I wanted to).
  • Changed the order of which the error and stack is shown with show_error() in all widget systems.

Edited for the sake of being kind.

>Codecov Report

Merging #1600 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1600   +/-   ##
  Coverage   18.53%   18.53%           
  Files         170      170           
  Lines       17080    17080           
  Hits         3165     3165           
  Misses      13915    13915

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b074d6b...e7c04dd. Read the comment docs.


@RobertBColton hey if Josh doesn't review this soon any chance if you could?

I will take a look at the pull request shortly now that you've cleaned up the pull request description. We didn't get to it because fundies flooded with the MSVC prs all at once, and while trivial and positive, they are time consuming to go through.

Ok, I think I picked this apart good enough and know what's being changed. Very nice work figuring that out because it's a really convoluted API on Microsoft's part! I'm really tired and I'll come back later and double check that it actually works. If so, then it's good to go.

Ok, I tested show_message(get_directory("holymoly", "captions")); which seems to work ok. The issues with the error dialog need fixed or reverted and split out into a separate pull before I will merge.

@RobertBColton I updated the description to include all changes made in the pr.

@RobertBColton alright, this one is ready to mudge, when you get the chance. thanks!
Please sign in to post comments, or you can view this issue on GitHub.