Feat#5688: Save As Profile option available for all tabs

[Clem-Fern]

Hi @Eugeny, I hope you are doing well !

I took a look on the feature request #5688 . This PR aims to make the 'Save As Profile' context menu item available for all tabs.

1. I saw that they were no ID setted with the new saved profile before. It causes the profile to not be shown in the Profiles & Connections selector since the commit c983743 : https://github.com/Eugeny/tabby/blob/c983743b57d90246d997b1ebf328a5c6696ac315/tabby-core/src/services/profiles.service.ts#L156 Should we create a recovery mechanism? Like creating an ID for profile those does not have one when the config is saved. What would be your approach on this?

2. During my test, I encountered the same issue as the one describe in #8534. https://github.com/Eugeny/tabby/blob/c983743b57d90246d997b1ebf328a5c6696ac315/tabby-core/src/services/config.service.ts#L200-L210 Even with JSON.parse(JSON.stringify(this._store)) and in rare case, some invalid yaml type seems to be present in the config object on yaml dump. Sadly I wasn't able to identify which part precisely... Anyway, adding skipInvalid: true in yaml dump option prevent the method to rise {name: 'YAMLException', reason: 'unacceptable kind of an object to dump [object Function]'} by skipping invalid type.

3. Also, It could be great to do a bit of refactoring in the future and create methods in the ProfileService for profile creation, edition and deletion to have only one entrypoint for profile operation. It would be simpler to maintain. What do you think ?

As always, feel free to ask me if changes needed :)

Resolve #5688

[Eugeny]

Wow - great job! Thank you for your continued efforts improving the app by the way :)

1. We can just assign random UUID IDs to any profiles that don't have one by adding a migration to ConfigService
2. Weird - the only way would be if some non-json-safe values are added again by maybeEncryptConfig ? Can't immediately think of any place this would happen in
3. Sounds great!

[Clem-Fern]

Thank's for your quick feedback. Well, I'm really glad to know that I'm able to help a bit with my small skills. I would like to help more but the greatest majority of issues stills absolutely out of my league for now ahah

1. Noted, I will add a commit for that in the next few days.

2. I think it stills a good idea to leave the skipInvalid: true option to have an additional safeguard. Be sure that I will take another look into this too.

3. Good! Expect a future PR ;)

[Clem-Fern]

Sorry for the delay,

1. As discuss, I added a new config migration to assign IDs to any profiles that don't have one. (Lint Job failed during Install Deps)
2. It's really weird... I was able to reproduce the issue #8534 almost all the time by following the path describe by @shankru5. Even with the code below in the save method, the YAMLException still raising. However, it seems that we do not encounter any error if the writing/parsing of the yaml is done with the yaml lib.

 async save (): Promise<void> { 
     await lastValueFrom(this.ready$) 
     if (!this._store) { 
         throw new Error('Cannot save an empty store') 
     } 
     // Scrub undefined values 
     let cleanStore = JSON.parse(JSON.stringify(this._store)) 
     // cleanStore = await this.maybeEncryptConfig(cleanStore) 
     await this.platform.saveConfig(yaml.dump(cleanStore)) 
     this.emitChange() 
 } 

Ready to merge from my side if you are ok with .1 and with the skipInvalid: true workaround for the .2.

[Eugeny]

Found it! Reading his console log, the error is in readRaw, not save.

readRaw is used to generate the config text for Settings -> Config pane and doesn't have the JSON roundtrip - can you add it in this PR? That would be perfect

[Clem-Fern]

Ohhh I see, great! I totally missed that... Thank's for the explanation