Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Save files as .js. Treat local .js files as SERVER_JS #27

Merged
merged 1 commit into from Feb 2, 2018

Conversation

jondcallahan
Copy link
Contributor

fixes #10

Save .gs files with the .js extension when fetching project files. Assign type SERVER_JS to local .js files when pushing to server.

@grant
Copy link
Contributor

grant commented Jan 25, 2018

Have you tested pushing two files, named test.gs and test.js? I think we need to handle this case. I think the API would fail if we do that.

What if someone pulls with the old version of clasp, and now pull with this new version of clasp? Won't they get duplicate files (different extensions)?

I think we'll have to break the workflow (users will have to manually rename files), in order to get this right. A potential solution we could implement is renaming all gs files to js before we push or pull. That way old gs files will be upgraded on push or pull.

What do you think?

@grant
Copy link
Contributor

grant commented Jan 25, 2018

I'd say, a reasonable thing to do would be to add some functionality that renames all .gs files to .js before pushing. That way old users don't have a breaking change and all future users will just use .js

@jondcallahan
Copy link
Contributor Author

How should we handle name collisions if there are files Code.js and Code.gs when renaming?

My initial approach of adding

if (path.extname(name).toLowerCase() === '.gs') {
    // Rename local .gs files to .js
    fs.rename(name, `${nameWithoutExt}.js`)
}

at line 487 inside readMultipleFiles will overwrite Code.js with Code.gs. Should the tool afford for this case, or blindly rename the .gs files to .js?

@grant
Copy link
Contributor

grant commented Jan 26, 2018

Let's fail pushing if there is a .gs and .js file of the same name. So:

  • clasp push

    • If gs/js conflict:
      • Prompt the user to rename .gs to .js:
        • clasp uses .js files for Apps Script. Rename all .gs files to .js (y/n)?
        • y: For all files in the clasp project, rename .gs files to .js
        • n/anything else: Log "Aborting push"
        • (Some GitHub repos have .gs files, we should provide a good experience with clasp still.)
    • If no gs/js conflict:
      • Rename and console.log a message Rename all .gs files to .js.
      • Push
  • clasp pull

    • If gs file found
      • console.log a message saying "Renamed XXX.gs to XXX.js"
      • Pull
    • If no gs/js conflict
      • Pull

clasp isn't really smart when pulling anyways (we don't merge, erase, or otherwise sync when pulling)

There are still lots of Apps Script projects with .gs like googlesamples/apps-script, and so we need to support .gs still.

I think users will have to deal with duplicate files no matter what due to previous versions.

@grant
Copy link
Contributor

grant commented Jan 28, 2018

@jondcallahan, Let me know if you want me to work on the logic of #27 (comment). I'm going to try to update clasp on npm with the latest PRs by Feb 5.

@jondcallahan
Copy link
Contributor Author

@grant Yes, I would appreciate you working on the logic, thanks. I dove in for a bit today but won't have time to pick it back up for a little while. One open question is how to handle the renaming after a user enters y. I think we should make it clear that Code.js will be overwritten with the contents of Code.gs or create a new file with a name like Code.gs.js to preserve the contents of Code.js.

@grant grant merged commit dd1ae46 into google:master Feb 2, 2018
@DimuDesigns
Copy link

I like the idea, but some users may want to retain a *.gs centric workflow. Maybe clasp needs to support a configuration mode where users can opt-in or opt-out of this feature. Maybe something like:
clasp config [CONFIG-FLAG] [SETTING-FOR-FLAG].

@grant
Copy link
Contributor

grant commented Mar 26, 2018

I'm open to PRs that add extension to .clasp.json and implement custom extensions (gs/js/ts).

sqrrrl pushed a commit that referenced this pull request May 11, 2021
…DME (#826) (#27)

Co-authored-by: Yash Totale <30784592+YashTotale@users.noreply.github.com>
Co-authored-by: Mattias Ekstrand <mattias.ekstrand@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IDE support? (IntelliJ, VS Code)
4 participants