Skip to content
This repository was archived by the owner on Aug 31, 2021. It is now read-only.

Fix initial setting of LCB editor pref#1869

Merged
montegoulding merged 5 commits intolivecode:develop-8.2from
bwmilby:bwm-bugfix-LCB_textEditor
Jan 10, 2018
Merged

Fix initial setting of LCB editor pref#1869
montegoulding merged 5 commits intolivecode:develop-8.2from
bwmilby:bwm-bugfix-LCB_textEditor

Conversation

@bwmilby
Copy link
Copy Markdown
Contributor

@bwmilby bwmilby commented Dec 18, 2017

The revIDEDeveloperExtensionEditScript handler will open an LCB file in the default editor. It uses the result to catch failures to open with a default editor and set a pref. The problem is that after evaluating the next put (getting the editor pref), the original value is lost. This means that if the pref has not already been set, then the file won't get opened.

This change implements a new variable tResult to carry forward the result though the function.

The `revIDEDeveloperExtensionEditScript` handler will open an LCB file in the default editor.  It uses `the result` to catch failures to open with a default editor and set a pref.  The problem is that after evaluating the next put (tEditor), the original value is lost.  This means that if the pref has not already been set, then the file won't get opened.

This change implements a new variable `tResult` to carry forward the result though the function.
end if

if the result is "no association" or the result is "request failed" then
if tResult is "no association" or the result is "request failed" then
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a second the result here that should be replaced

@livecodeali
Copy link
Copy Markdown
Member

@bwmilby thanks for this!

Corrected remaining `the result` to `tResult`
@bwmilby
Copy link
Copy Markdown
Contributor Author

bwmilby commented Dec 18, 2017

@livecodeali Glad to help. Not sure how much help this will need merging into 9 since this code was factored out to a new function call.


if the result is not empty then
if tResult is not empty then
__revIDEDeveloperExtensionSendError "Could not open" && tFile & ":" && the result
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, missed one here too!

@livecodeali
Copy link
Copy Markdown
Member

Actually, do we think that the LCB_textEditor preference should take precedence over file association? I think @montegoulding has suggested this in the past.

Corrected remaining `the result` to `tResult`
__revIDEDeveloperExtensionSendError "Could not open" && tFile & ":" && tResult
else if it is not empty then
# AL-2015-04-01: [[ Bug 15130 ]] If the new text editor launch was successful, set the preference
revIDESetPreference "LCB_textEditor", it
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to see this moved up to after line 600 with:

if tResult is empty then
        # AL-2015-04-01: [[ Bug 15130 ]] If the new text editor launch was successful, set the preference
        revIDESetPreference "LCB_textEditor", it
end if

I don't feel safe that it will be empty if there were already an association.

Relocate storage of new text editor preference to immediately after the launch to ensure `it` contains the intended value
@bwmilby
Copy link
Copy Markdown
Contributor Author

bwmilby commented Dec 19, 2017

If the preference were to take precedence over the file association, then I think there needs to be a way to set/reset the pref in the IDE somewhere (probably a good idea anyway). Could be a menu option on the cog of the Extension Builder stack (palletActions widget) since this is the only place it is used.

@livecodeali
Copy link
Copy Markdown
Member

Yeah, that's a good idea. I'm not sure exactly what the best way of populating the list of possible programs would be though.

@bwmilby
Copy link
Copy Markdown
Contributor Author

bwmilby commented Dec 19, 2017

Doing some testing with the additions and found an issue. The result is being set to the app name.

The following command sequence (in message box):
`put revIDEGetPreference("LCB_textEditor") into tEditor; launch ".../treeviewx.lcb" with tEditor; put the result`
Results in the value of `tEditor` when successful (although success in opening the file is not guaranteed)

Added a check for "no such program" in the case where the specified editor is no longer available
Updated code to use the `tEditor` variable in place of `it` for clarity
Added a check for `tResult` being `tEditor` to not display an error
@montegoulding
Copy link
Copy Markdown
Contributor

@livecode-vulcan review ok 9a63f32

@livecode-vulcan
Copy link
Copy Markdown

💙 review by @montegoulding ok 9a63f32

@livecodeali livecodeali modified the milestones: 9.0.0-rc-1, 8.2.0-dp-3 Jan 8, 2018
@montegoulding montegoulding merged commit 7cff5f4 into livecode:develop-8.2 Jan 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants