Skip to content

Adding c/d/h variants of insertLink, echoPopup, and insertPopup#3009

Merged
demonnic merged 13 commits intoMudlet:developmentfrom
demonnic:c-d-h-popups
Aug 22, 2019
Merged

Adding c/d/h variants of insertLink, echoPopup, and insertPopup#3009
demonnic merged 13 commits intoMudlet:developmentfrom
demonnic:c-d-h-popups

Conversation

@demonnic
Copy link
Copy Markdown
Member

Brief overview of PR changes/additions

adds the c/d/h versions of the insertLink, echoPopup, and insertPopup functions. Already have echo, insertText and echoLink so I figured I should add the rest when I went to use cechoPopup and discovered it didn't exist yet.

Motivation for adding to Mudlet

True up API. Surprised nobody has asked for these before (that I could find with a brief search)

Other info (issues closed, discussion etc)

@demonnic demonnic requested a review from a team August 19, 2019 22:56
@add-deployment-links
Copy link
Copy Markdown

add-deployment-links bot commented Aug 19, 2019

Hey there! Thanks for helping Mudlet improve. 🌟

Test versions

You can directly test the changes here:

No need to install anything - just unzip and run.
Let us know if it works well, and if it doesn't, please give details.

@SlySven
Copy link
Copy Markdown
Member

SlySven commented Aug 19, 2019

There has been a change to the edbee-lib widget sub-module - I think we may even have changed the source repository - whilst locally that can be pulled in with a git submodule sync I am not sure how/when/if this has been done to the CI infrastructure - and it looks like it hasn't - or your PR is not based off of the right starting point and is thus failing CI now 🤷‍♂️ ...

@SlySven
Copy link
Copy Markdown
Member

SlySven commented Aug 19, 2019

🔍 Oooh, it looks to me as though the commit bdd66b1 may have reverted the edbee-lib submodule back to the old one... 😮

@demonnic
Copy link
Copy Markdown
Member Author

good eye, not sure how that even got introduced. I'll revert that changeset and reapply the actual lua changes

@demonnic
Copy link
Copy Markdown
Member Author

Should be good now

@demonnic
Copy link
Copy Markdown
Member Author

Realized Geyser didn't even have all the functions we already had setup, let alone the ones in this PR, so I added a bunch of geyser wrappers too.

Copy link
Copy Markdown
Member

@vadi2 vadi2 left a comment

Choose a reason for hiding this comment

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

cinsertPopup("<red>activities<reset> to do", {[[send "sleep"]], [[send "sit"]], [[send "stand"]]}, {"sleep", "sit", "stand"}) doesn't seem to do anything, just eats the command.

insertLink(self.name, ...)
end

--- inserts clickable text with right-click menu into the miniconsole at the end of the current line
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.

These will appear in the Geyser ldoc pages but don't give any real guidance on how to be used... but this is an issue with existing functions above too.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

They're largely wrappers around already documented stuff in the wiki, I could run through and include links to the wiki for the base functions they wrap

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.

That would be great.

@demonnic
Copy link
Copy Markdown
Member Author

I'll check cinsertPopup in particular in a moment here.

@demonnic
Copy link
Copy Markdown
Member Author

So it turns out this is because insertPopup eats the command if you pass "main" as the window name, rather than acting against the main window. So it looks like this is a bug in insertPopup.

@vadi2
Copy link
Copy Markdown
Member

vadi2 commented Aug 21, 2019

OK, I'll get that fixed first, since otherwise without that this PR isn't of super quality.

@vadi2
Copy link
Copy Markdown
Member

vadi2 commented Aug 21, 2019

OK, check out #3013.

@vadi2
Copy link
Copy Markdown
Member

vadi2 commented Aug 21, 2019

OK, with that issue fixed, update this branch and I'll re-test.

@demonnic
Copy link
Copy Markdown
Member Author

Alright, links to mudlet wiki for base functions added, merged upstream development back into this branch to pick up fix.

@demonnic demonnic merged commit a6b185e into Mudlet:development Aug 22, 2019
@demonnic demonnic deleted the c-d-h-popups branch August 22, 2019 01:11
dicene pushed a commit to dicene/Mudlet that referenced this pull request Feb 19, 2020
…et#3009)

* Adding c/d/h variants of insertLink, echoPopup, and insertPopup
* Adding c/d/h versions of echoLink, insertLink, echoPopup, and insertPopup to Geyser.MiniConsole
* Adding urls to the wiki for c/d/h insert/echoLink/Popup wrapper functions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants