Added an optional user modifiable default sketch file when creating a new project. #1559

nbourre · 2022-10-14T01:19:11Z

Motivation

Sometimes programmers use the same boilerplate code for their project. This PR allows the creation of a default sketch.

Change description

If a default/default.ino file is present in the Arduino Sketch folder, the app will read the content of the default.ino file to create the new project. If the file is non-existent, the default hard-coded sketch is used.

Other information

I created a loadDefault() method in the sketches-service-impl.ts file.

Reviewer checklist

PR addresses a single concern.
The PR has no duplicates (please search among the Pull Requests before creating one)
PR title and description are properly filled.
Docs have been added / updated (for bug fixes / features)

CLAassistant · 2022-10-14T01:19:18Z

All committers have signed the CLA.

nmzaheer · 2022-10-16T05:08:23Z

This needs to be documented somewhere for user discoverability.

As a further change, we could add a settings parameter that points to a path of the template sketch in a folder marked for templates. This would make managing of templates easier.

per1234 · 2022-10-16T10:53:07Z

This needs to be documented

Excellent point @nmzaheer. I would suggest the "Advanced Usage" document as the appropriate place for such information:

https://github.com/arduino/arduino-ide/blob/main/docs/advanced-usage.md

However, that would not be necessary if your other suggestion was implemented:

add a settings parameter

In this case, I think the existence of the setting and the accompanying description would be sufficient to make the feature discoverable to the user.

nmzaheer · 2022-10-16T11:08:12Z

However, that would not be necessary if your other suggestion was implemented:

So are you suggesting that this change should not be merged and rather we work a bit more to add it to the Settings ?

nbourre · 2022-10-17T13:48:56Z

Could it be an iterative update. Such as in the first iteration, it is not in the UI but only in the documentation and in a second iteration, the UI is available in an advanced settings?

I'm really not a UI/web guy. I'm mainly from the C/C# world and I don't feel comfortable making change to the UI.

kittaakos · 2022-10-17T17:40:05Z

Could it be an iterative update.

No question we need them together.

I don't feel comfortable making change to the UI.

You do not have to. Checking your changeset, I am positive you can do it. Here are a few pointers:

If you want to add support for a new setting, you need to update the "schema", and the framework will generate the rest. Nice, isn't it? You have to change /arduino-ide-extension/src/browser/arduino-preferences.ts here:

diff --git a/arduino-ide-extension/src/browser/arduino-preferences.ts b/arduino-ide-extension/src/browser/arduino-preferences.ts
index 5fef5907..9e9b3f9d 100644
--- a/arduino-ide-extension/src/browser/arduino-preferences.ts
+++ b/arduino-ide-extension/src/browser/arduino-preferences.ts
@@ -241,6 +241,14 @@ export const ArduinoConfigSchema: PreferenceSchema = {
       ),
       default: false,
     },
+    'arduino.sketch.inoBlueprint': {
+      type: 'string',
+      description: nls.localize(
+        'arduino/preferences/sketch/inoBlueprint',
+        'Absolute filesystem path to the default `.ino` blueprint file. If specified, the content of the blueprint file will be used for every new sketch created by the IDE. The sketches will be generated with the default Arduino content if not specified. Unaccessible blueprint files are ignored. A restart of the IDE is needed for this setting to take effect.'
+      ),
+      default: undefined,
+    },
     'arduino.checkForUpdates': {
       type: 'boolean',
       description: nls.localize(
@@ -278,6 +286,7 @@ export interface ArduinoConfiguration {
   'arduino.auth.registerUri': string;
   'arduino.survey.notification': boolean;
   'arduino.cli.daemon.debug': boolean;
+  'arduino.sketch.inoBlueprint': string;
   'arduino.checkForUpdates': boolean;
 }

Next task is to read and use this value from the settings. Usually, these things happen in the browser window through a proper service, but we need to use a hack here. No worries, we did this once. Here is an example:

arduino-ide/arduino-ide-extension/src/node/arduino-daemon-impl.ts

Lines 174 to 194 in 99b1094

    
           private async debugDaemon(): Promise<boolean> { 
        
             // Poor man's preferences on the backend. (https://github.com/arduino/arduino-ide/issues/1056#issuecomment-1153975064) 
        
             const configDirUri = await this.envVariablesServer.getConfigDirUri(); 
        
             const configDirPath = FileUri.fsPath(configDirUri); 
        
             try { 
        
               const raw = await fs.readFile(join(configDirPath, 'settings.json'), { 
        
                 encoding: 'utf8', 
        
               }); 
        
               const json = this.tryParse(raw); 
        
               if (json) { 
        
                 const value = json['arduino.cli.daemon.debug']; 
        
                 return typeof value === 'boolean' && !!value; 
        
               } 
        
               return false; 
        
             } catch (error) { 
        
               if ('code' in error && error.code === 'ENOENT') { 
        
                 return false; 
        
               } 
        
               throw error; 
        
             } 
        
           }

Make sure you read the content of the blueprint file once and save it as a field to the SketchesService. Currently, you read it every time you invoke loadDefault. It's not good. State in the description of the new setting that IDE2 needs a restart. It's OK for now.
I am OK ignoring all errors when reading the blueprint file (fs.readFile), but if the code is not ENOENT log it to the console at least. See the example above.
When you're done with the development, run yarn i18n:generate from the repo's root to update the translation files with the description of the new setting. Commit the changed ./i18n/en.json file.
I recommend you install dbaeumer.vscode-eslint VS Code extension if you're using Code. It will clean up all styling issues, such as let raw = ""; should be let raw = '';

Keep up the great work, and let me know if you have any questions.

kittaakos · 2022-10-20T15:30:09Z

We should probably consider the default content for the remote sketches:

arduino-ide/arduino-ide-extension/src/browser/create/create-api.ts

Lines 521 to 533 in 93291b6

    
             export const defaultInoContent = `/* 
        
           */ 
        
           void setup() { 
        
           } 
        
           void loop() { 
        
           } 
        
           `;

kittaakos · 2022-10-21T07:09:55Z

@nbourre, do you plan to make the requested changes? We want to release a new patch version of IDE2 soon and include this feature.

nmzaheer · 2022-10-21T07:53:33Z

@kittaakos I would like to take a look at this. How do I contribute without creating a new PR? Should I open a PR in @nbourre repo and if that is merged then it would be reflected in this PR ?

On a side note, I think we need to start tagging issues to milestones i.e. 2.01, 2.02, etc. for contributors to know what i being prioritized by the development team.

nbourre · 2022-10-21T12:07:50Z

@kittaakos What is the ETA for the next release? I'm quite tight in my time today. I might be able to do it this weekend.

@nmzaheer I will try to work on it this weekend, but if you have time and are fluent with this framework, feel free to do a PR on my repo.

kittaakos · 2022-10-21T12:18:55Z

@kittaakos What is the ETA for the next release?

Thank you for the quick reply. I plan to merge everything by latest next Tuesday (25.10).

If your schedule is tight, I am happy to add support for the settings.json as a separate commit and keep you as the original author of the PR. I am also glad to answer all your questions if you plan to solve the task alone.

You're very close to the final solution, and I think you can do it. 💪 But no pressure, of course.

nbourre · 2022-10-22T18:57:14Z

Hi guys,

Link to my last commit : 6444edc

I'm having trouble to make the modification work.

By @kittaakos, here are the task and what I've done so far :

But I don't see the settings in the preferences. What am I missing?

The commands I've done :

yarn
yarn rebuild:browser
yarn rebuild:electron
yarn start

Do you guys have a cue to improve the build speed? Building the apps takes an eternity compared to C-style apps.

Thanks

nbourre · 2022-10-22T19:06:09Z

Ok, I got it. The default folder is going to be in the advanced Preferences not the one accessible by the menu.

nbourre · 2022-10-23T12:26:21Z

@kittaakos I think I have accomplished all the requirements for the next release.

kittaakos

Ok, I got it. The default folder is going to be in the advanced Preferences not the one accessible by the menu.

There is no default folder. Users either give an absolute path to the default .ino content file or the default will be used from the code. But IDE2 won't try to pick up any files from #directories.user/default/default.ino.

Please check your template, you have added undesired whitespaces. See here.

I simplified the code. Here is my proposal: nbourre/arduino-ide@main...kittaakos:arduino-ide:default-ino-review

Feel free to use it.

arduino-ide-extension/src/browser/arduino-preferences.ts

kittaakos · 2022-10-24T08:19:17Z

arduino-ide-extension/src/node/sketches-service-impl.ts

+  // put your setup code here, to run once:
+
+}
+
+void loop() {
+  // put your main code here, to run repeatedly:
+
+}`;


There are undesired whitespaces in the template.

arduino-ide-extension/src/node/sketches-service-impl.ts

nbourre · 2022-10-25T02:18:12Z

@kittaakos, except for the description which I'm not sure what's wrong. I've applied the requested changes.

kittaakos · 2022-10-25T06:35:39Z

except for the description which I'm not sure what's wrong.

The indentation of the code was incorrect, but you have fixed it. Thank you!

All looks good except the default template.

If the new arduino.sketch.inoBlueprint setting is not defined or pointing to an inaccessible file, the default .ino content looks like this using the build from this PR:

void setup() {
  // put your setup code here, to run once:
}
void loop() {
  // put your main code here, to run repeatedly:
}

Expected:

void setup() {
  // put your setup code here, to run once:

}

void loop() {
  // put your main code here, to run repeatedly:

}

missing empty line after comment inside the method,
missing empty line between methods, and
missing newline at the end of the file.

Here is my proposal: nbourre/arduino-ide@main...kittaakos:arduino-ide:default-ino-review

Please reference my proposed template here. If you fix the .ino content, this PR will be ready for merge.

kittaakos

It's working great. Thank you so much for your contribution, @nbourre 👍

kittaakos · 2022-10-26T12:07:57Z

I have also verified on Windows from the build. ✨

UKHeliBob · 2022-10-26T18:24:09Z

This seems to have been made more complicated that needed by adding a new setting. It is too late now, but what was the problem with the original proposal to use a sketch named default,ino if it existed rather than adding a new setting ?

Come to that you could simply have used the same mechanism as the classic IDE and used the BareMinimum example as the new sketch template

Anyone who felt the need for a different default sketch, as I did, would simply edit the BareMinimum example to suit themselves

per1234 · 2022-10-26T22:52:23Z

Hi @UKHeliBob

This seems to have been made more complicated that needed by adding a new setting.

I don't see any problem with the feature being slightly complex to set up. This is something only a small portion of advanced users will do, and something they will do only once.

The important thing is to provide the capability, and to provide that capability in a manner that can meet all user's needs, not in streamlining the feature as would be more of a consideration for some operation that is being done repeatedly or by beginners.

what was the problem with the original proposal to use a sketch named default,ino if it existed

Because some users might create a sketch of this name without any intention of it being adopted by the IDE as the base sketch.

Because some users might prefer to store the sketch in some other location or under a different name.

you could simply have used the same mechanism as the classic IDE and used the BareMinimum example as the new sketch template

The reimplementation of each feature in Arduino IDE 2.x is a chance to reconsider the old one and improve on it where areas for improvement are identified. This was one with significant areas for improvement, and those who collaborated here on the feature took the time and effort to make it better!

With the old system, your custom new sketch was lost at every IDE update.

With some operating systems, packages, security configurations, it is impossible to edit files inside the Arduino IDE installation package.

Some users may have different requirements for the new sketch than they do for the sketch accessed via File > Examples > 01.Basics > BareMinimum example sketch.

UKHeliBob · 2022-10-27T08:02:56Z

This is something only a small portion of advanced users will do,

That is exactly the point. If the BareMinimum sketch were used as the new sketch template then advanced users could simply modify it to meet their needs as they do now

My Windows setup prevents me from modifying the example sketches and saving them again using the IDE, but being an "advanced user" that was easily circumvented

I cannot comment on the comparative difficulty of implementing the change either way because I am not familiar with the programming environment

I assume that the new mechanism will appear in a nightly build soon, if it has not done so already and I look forward to trying it out

nmzaheer · 2022-10-27T09:44:20Z

The whole point of IDE 2 is that it will have breaking changes compared to IDE 1. However, the goal is not to alienate the entire userbase by creating a user experience which is vastly different from IDE 1 since that is the targeted audience.

As pointed out by @per1234 , the Bare minimum sketch gets overwritten with every update of Arduino IDE 2 which does not ensure a good UX since the user will have to keep changing contents with every update. And I am assuming till features and 🐛stabilize, the team would be looking at having release every month or so.

So, I feel having a setting to a default sketch would be a good UX rather than editing the BareMinimum sketch.

On a side note, maybe the team could also discuss having a menu item under File with New Sketch from Template as an option. However, we will have to set up some sort of data collection to understand how many use that option and if it deserves such high visibility under the File menu

nbourre added 2 commits October 13, 2022 21:05

Added a modifiable default sketch for new project

d28398e

Removed unused file

f41c1cd

per1234 linked an issue Oct 14, 2022 that may be closed by this pull request

Make 'bare minimum' sketch modifiable by users #373

Closed

per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Oct 14, 2022

per1234 added the status: changes requested Changes to PR are required before merge label Oct 20, 2022

WiP : Now nothing's working... :(

6444edc

yarn i18n:generate for the settings

9169e20

nbourre added 2 commits October 22, 2022 15:35

Updated the desription for markdown description.

2190f81

Lintered the code

12ab5d9

per1234 removed the status: changes requested Changes to PR are required before merge label Oct 23, 2022

kittaakos self-requested a review October 24, 2022 08:18

kittaakos suggested changes Oct 24, 2022

View reviewed changes

kittaakos added the status: changes requested Changes to PR are required before merge label Oct 24, 2022

nbourre added 2 commits October 24, 2022 22:01

Remove undesirable whitespaces

016eb2a

Applied kittaakos suggestions

f38ee10

Removed extra whitespaces

558a71e

kittaakos removed the status: changes requested Changes to PR are required before merge label Oct 25, 2022

kittaakos added the status: changes requested Changes to PR are required before merge label Oct 25, 2022

Fixed default .ino for the missings empty lines.

4f2010e

nbourre requested a review from kittaakos October 25, 2022 19:13

per1234 removed the status: changes requested Changes to PR are required before merge label Oct 26, 2022

kittaakos approved these changes Oct 26, 2022

View reviewed changes

kittaakos merged commit 0773c39 into arduino:main Oct 26, 2022

kittaakos mentioned this pull request May 16, 2023

Default sketch names sort out of order #2068

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added an optional user modifiable default sketch file when creating a new project. #1559

Added an optional user modifiable default sketch file when creating a new project. #1559

nbourre commented Oct 14, 2022

CLAassistant commented Oct 14, 2022 •

edited

Loading

nmzaheer commented Oct 16, 2022

per1234 commented Oct 16, 2022

nmzaheer commented Oct 16, 2022

nbourre commented Oct 17, 2022 •

edited

Loading

kittaakos commented Oct 17, 2022 •

edited

Loading

kittaakos commented Oct 20, 2022

kittaakos commented Oct 21, 2022

nmzaheer commented Oct 21, 2022

nbourre commented Oct 21, 2022

kittaakos commented Oct 21, 2022

nbourre commented Oct 22, 2022 •

edited

Loading

nbourre commented Oct 22, 2022

nbourre commented Oct 23, 2022

kittaakos left a comment

kittaakos Oct 24, 2022

nbourre commented Oct 25, 2022

kittaakos commented Oct 25, 2022

kittaakos left a comment

kittaakos commented Oct 26, 2022

UKHeliBob commented Oct 26, 2022

per1234 commented Oct 26, 2022

UKHeliBob commented Oct 27, 2022

nmzaheer commented Oct 27, 2022

Added an optional user modifiable default sketch file when creating a new project. #1559

Added an optional user modifiable default sketch file when creating a new project. #1559

Conversation

nbourre commented Oct 14, 2022

Motivation

Change description

Other information

Reviewer checklist

CLAassistant commented Oct 14, 2022 • edited Loading

nmzaheer commented Oct 16, 2022

per1234 commented Oct 16, 2022

nmzaheer commented Oct 16, 2022

nbourre commented Oct 17, 2022 • edited Loading

kittaakos commented Oct 17, 2022 • edited Loading

kittaakos commented Oct 20, 2022

kittaakos commented Oct 21, 2022

nmzaheer commented Oct 21, 2022

nbourre commented Oct 21, 2022

kittaakos commented Oct 21, 2022

nbourre commented Oct 22, 2022 • edited Loading

nbourre commented Oct 22, 2022

nbourre commented Oct 23, 2022

kittaakos left a comment

Choose a reason for hiding this comment

kittaakos Oct 24, 2022

Choose a reason for hiding this comment

nbourre commented Oct 25, 2022

kittaakos commented Oct 25, 2022

kittaakos left a comment

Choose a reason for hiding this comment

kittaakos commented Oct 26, 2022

UKHeliBob commented Oct 26, 2022

per1234 commented Oct 26, 2022

UKHeliBob commented Oct 27, 2022

nmzaheer commented Oct 27, 2022

CLAassistant commented Oct 14, 2022 •

edited

Loading

nbourre commented Oct 17, 2022 •

edited

Loading

kittaakos commented Oct 17, 2022 •

edited

Loading

nbourre commented Oct 22, 2022 •

edited

Loading