Files icon indicating copy to clipboard operation
Files copied to clipboard

Feature: Display error message when settings file isn't valid

Open yaira2 opened this issue 5 years ago • 19 comments

If the json file isn't valid, we should notify the user of the issue.

Requirements

  • Display modal if the settings file isn't valid with a description of the issue
  • Include button to reset settings
  • Include button to close app

yaira2 avatar Apr 27 '20 15:04 yaira2

As i told in #562 we should unify all exceptions(replace JsonSerializationException with Exception or add JsonReaderException), so if there will be any json parsing error, just reset this json to default.

tsvietOK avatar Apr 27 '20 15:04 tsvietOK

But if we reset the json file, the user loses all his profiles and we will have to store a default json file. If the terminal loading process goes wrong for whatever reason, i would simply default to cmd and log, that there was an error. So the user can investigate his json file, if he want's to fix it and if not, he gets the cmd terminal.

lampenlampen avatar Apr 28 '20 11:04 lampenlampen

That would be a nice addition, however it would still make sense to include a json file for someone who wants to use the json format.

yaira2 avatar Apr 28 '20 14:04 yaira2

Maybe we should get rid of the json file, especially if we auto-detect the common terminals (powershell (core), WindowsTerminal).

And for the few users, who want's to use a custom one, they can choose custom and then we present them an input box to specify the path.

I choosed json, because i thought then they can specify multiple terminals and icons and so on, but i don't think we will use that customization. If a user want's control over multiple terminal, he will install Windows Terminal or terminus, ...

lampenlampen avatar Apr 28 '20 15:04 lampenlampen

Maybe we should get rid of the json file, especially if we auto-detect the common terminals (powershell (core), WindowsTerminal).

And for the few users, who want's to use a custom one, they can choose custom and then we present them an input box to specify the path.

I choosed json, because i thought then they can specify multiple terminals and icons and so on, but i don't think we will use that customization. If a user want's control over multiple terminal, he will install Windows Terminal or terminus, ...

@lampenlampen that is a possibility, and once support is added for importing and exporting the settings json file, users can technically add a custom terminal path even it's not detected automatically by Files.

yaira2 avatar Oct 10 '21 17:10 yaira2

It's not been resolved yet. I'm working on this. Simple try-catch should be enough.

d2dyno1 avatar Nov 11 '21 17:11 d2dyno1

I think this issue should be modified to not conflict with #10935, and leaving this as just the error modal and reset

hecksmosis avatar Jan 21 '23 14:01 hecksmosis

I updated the requirements as per @hecksmosis feedback.

yaira2 avatar Mar 28 '23 23:03 yaira2

Hey all! If no one's currently working on this, I want a go at implementing this feature! It'll be my first attempt at contributing to an open-source project, so I'm pretty excited honestly! -M

ClockWorkElementals avatar Apr 08 '23 06:04 ClockWorkElementals

Sure go ahead

Josh65-2201 avatar Apr 08 '23 08:04 Josh65-2201

@ClockWorkElementals I assigned you 🙂

d2dyno1 avatar Apr 08 '23 21:04 d2dyno1

Heads up that I am looking into this.

HorseBandit avatar Sep 08 '23 01:09 HorseBandit

Heads up that I am looking into this.

If Bandit's taking a shot, feel free to give it to em! I couldn't quite figure my way around everything while taking my shot

ClockWorkElementals avatar Sep 08 '23 01:09 ClockWorkElementals

What's the status of this issue?

zackatack108 avatar Nov 08 '23 20:11 zackatack108

Available for work

Get Outlook for iOShttps://aka.ms/o0ukef


From: zackatack108 @.> Sent: Wednesday, November 8, 2023 12:11:39 PM To: files-community/Files @.> Cc: Estell, Jerrod Victor @.>; Comment @.> Subject: Re: [files-community/Files] Feature: Display error message when settings file isn't valid (#710)

[EXTERNAL EMAIL]

What's the status of this issue?

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https://github.com/files-community/Files/issues/710*issuecomment-1802578161__;Iw!!JmPEgBY0HMszNaDT!oDmUjHFn6i4OFuPxQzgY6yUGIpgByVeCcOk8PgXRCWfVYsSf7luCyZXTLiR9Tvx4taBWc59o6sqGN6krf0kbCbKh8bxL$, or unsubscribehttps://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/A35NWDLJXEXBAS3FZKOGESDYDPRPXAVCNFSM4MSAOJVKU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOBQGI2TOOBRGYYQ__;!!JmPEgBY0HMszNaDT!oDmUjHFn6i4OFuPxQzgY6yUGIpgByVeCcOk8PgXRCWfVYsSf7luCyZXTLiR9Tvx4taBWc59o6sqGN6krf0kbCS6bh5EC$. You are receiving this because you commented.Message ID: @.***>

HorseBandit avatar Nov 08 '23 20:11 HorseBandit

It hasn't been implemented yet

yaira2 avatar Nov 08 '23 22:11 yaira2

I might implement this feature in a week or something but I'm not sure if I will get it working. I've tried doing something with it today but to no avail. There's so much stuff involved with creating a new dialog and making it work. I'll try to do a research on this one and come up with a solution.

If I'm not done within a week or so then it would probably mean that I stopped working on the issue and it's free for taking.

RieBi avatar Nov 18 '23 14:11 RieBi

Concept from Windows Terminal: image

0x5bfa avatar Dec 22 '23 06:12 0x5bfa

I was working on this and make it work correctly but current implementation of json serializer for settings is not intended to display UI from their own classes and those classes are exposed as abstract classes, which means I should've add an event to notify of the parse error to AppModel after UI is loaded to prevent the dialog from having to be tried to show before UI thread starts.

@mdtauk Do you have design concept or have something to tell me before working on again, such as adding what buttons and displaying what messages?

0x5bfa avatar Apr 17 '24 06:04 0x5bfa