raspberrypi / pico-project-generator

Tool to automatically generate a Pico C SDK Project
BSD 3-Clause "New" or "Revised" License
289 stars 74 forks source link

Fix #84, SyntaxError in settings.json #85

Closed paulober closed 1 year ago

paulober commented 1 year ago
JamesH65 commented 1 year ago

I'd usually prefer not to have the whitespace changes at the same time, but think its OK for this one. Thanks!

paulober commented 1 year ago

I'd usually prefer not to have the whitespace changes at the same time, but think its OK for this one. Thanks!

Sorry, I would normally split these things up too, at least into different commits, but this is the first time I've contributed to this repo and the last 2 PRs had very small changes so I didn't want to bother. Next time I will split these things into different commits or PRs.

lurch commented 12 months ago

Rather than using single-quoted strings, into which you manually have to insert all the \n escapes, perhaps it'd be easier to just use triple-quoted strings, and then you could simply copy'n'paste the file-contents directly? I couldn't find a definitive reference, so have a browse through some of https://www.google.com/search?q=python+triple-quoted+string

But basically you could replace

            settings = ( '{\n'
                   '  "cmake.statusbar.advanced": {\n'
                   '    "debug": {\n'
                   '      "visibility": "hidden"\n'
                   '    },\n'
                   '    "launch": {\n'
                   '      "visibility": "hidden"\n'
                   '    },\n'
                   '    "build": {\n'
                   '      "visibility": "hidden"\n'
                   '    },\n'
                   '    "buildTarget": {\n'
                   '      "visibility": "hidden"\n'
                   '    }\n'
                   '  },\n'
                   '  "cmake.buildBeforeRun": true,\n'
                   '  "cmake.configureOnOpen": true,\n'
                   '  "cmake.configureSettings": {\n'
                   '    "CMAKE_MODULE_PATH": "${env:PICO_INSTALL_PATH}/pico-sdk-tools"\n'
                   '  },\n'
                   '  "cmake.generator": "Ninja",\n'
                   '  "C_Cpp.default.configurationProvider": "ms-vscode.cmake-tools"\n'
                   '}\n')

with


            settings = """{
  "cmake.statusbar.advanced": {
    "debug": {
      "visibility": "hidden"
    },
    "launch": {
      "visibility": "hidden"
    },
    "build": {
      "visibility": "hidden"
    },
    "buildTarget": {
      "visibility": "hidden"
    }
  },
  "cmake.buildBeforeRun": true,
  "cmake.configureOnOpen": true,
  "cmake.configureSettings": {
    "CMAKE_MODULE_PATH": "${env:PICO_INSTALL_PATH}/pico-sdk-tools"
  },
  "cmake.generator": "Ninja",
  "C_Cpp.default.configurationProvider": "ms-vscode.cmake-tools"
}
"""
paulober commented 12 months ago

@lurch You're right, that would be much more convenient and errors like the additional "," would have been spotted earlier without the \n, single quotes and + each line. I'll work on it and then provide a PR. Would then there be a consideration of merging the feature branch into master? (and maybe rename master to main ;))