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

Show fs error in paint and edit #390

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@Wilma456
Copy link
Contributor

Wilma456 commented Jul 28, 2017

If saving failed, you now get a error, that shows why it failed.

2017-07-28_15 30 41
2017-07-28_15 27 02

@@ -93,7 +93,7 @@ local function save( _sPath )
-- Save
local file = nil
local function innerSave()
file = fs.open( _sPath, "w" )
file, fileerr = fs.open( _sPath, "w" )

This comment has been minimized.

@Lignum

Lignum Jul 28, 2017

Contributor

fileerr should not be global.

@@ -107,7 +107,7 @@ local function save( _sPath )
if file then
file.close()
end
return ok, err
return ok, err, fileerr

This comment has been minimized.

@SquidDev

SquidDev Jul 28, 2017

Contributor

Perhaps instead of storing fileerr in an upvalue and returning, you should have error(fileerr, 0). That way you don't need the separate err and fileerr values. It would be worth checking how it handles out of space errors though - those will be generated through file.write instead.

@SquidDev

This comment has been minimized.

Copy link
Contributor

SquidDev commented Jul 29, 2017

It's worth considering how this will interact with #377 - I'm presuming you'll get something like Error saving to bild.nfp: (/bild.nfp: Out of space), which could be a little weird. Obviously we should wait for that to be merged before worrying about it though :).

@lupus590

This comment has been minimized.

Copy link
Contributor

lupus590 commented Jul 29, 2017

@SquidDev Thinking ahead for that case, should the programs modify the error message to remove the file name from the error (the part in brackets)?

@SquidDev

This comment has been minimized.

Copy link
Contributor

SquidDev commented Aug 30, 2017

@Wilma456 What does this look like now that #377 has been merged? Would it be better to no longer show the file name, as it'll be included in the fs error message?

@Wilma456

This comment has been minimized.

Copy link
Contributor Author

Wilma456 commented Aug 30, 2017

Here's the new Style
2017-08-30_17 21 43

@SquidDev SquidDev referenced this pull request Aug 15, 2018

Open

Upgrade the event #567

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.