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

Some messages and notifications are missing #42

Open
Ekopalypse opened this issue Oct 13, 2019 · 14 comments
Open

Some messages and notifications are missing #42

Ekopalypse opened this issue Oct 13, 2019 · 14 comments
Assignees
Labels

Comments

@Ekopalypse
Copy link
Contributor

@Ekopalypse Ekopalypse commented Oct 13, 2019

List of messages and notifications which aren't documented yet
NPPM_ADDTOOLBARICON
NPPM_DECODESCI
NPPM_DISABLEAUTOUPDATE
NPPM_DMMVIEWOTHERTAB
NPPM_DOCSWITCHERDISABLECOLUMN
NPPM_ENCODESCI
NPPM_GETCURRENTVIEW
NPPM_GETEDITORDEFAULTBACKGROUNDCOLOR
NPPM_GETEDITORDEFAULTFOREGROUNDCOLOR
NPPM_GETENABLETHEMETEXTUREFUNC
NPPM_GETFILENAMEATCURSOR
NPPM_GETNBUSERLANG
NPPM_GETNPPFULLFILEPATH
NPPM_GETPLUGINHOMEPATH
NPPM_MAKECURRENTBUFFERDIRTY
NPPM_REMOVESHORTCUTBYCMDID
NPPM_SAVEFILE
NPPM_SAVESESSION
NPPM_SETEDITORBORDEREDGE
NPPM_SETSMOOTHFONT
NPPM_SETSTATUSBAR

NPPN_BEFORESHUTDOWN
NPPN_CANCELSHUTDOWN
NPPN_FILEBEFOREDELETE
NPPN_FILEBEFORERENAME
NPPN_FILEDELETED
NPPN_FILEDELETEFAILED
NPPN_FILERENAMECANCEL
NPPN_FILERENAMED
NPPN_READONLYCHANGED
NPPN_SNAPSHOTDIRTYFILELOADED

@Ekopalypse

This comment has been minimized.

Copy link
Contributor Author

@Ekopalypse Ekopalypse commented Oct 13, 2019

Q: Is it assumed that I do a PR for each of the missing messages/notifications or
can I create one PR for all?

@pryrt

This comment has been minimized.

Copy link
Contributor

@pryrt pryrt commented Oct 13, 2019

A: just do the one PR for a family of similar changes

@pryrt pryrt closed this Oct 13, 2019
@pryrt pryrt reopened this Oct 13, 2019
@Ekopalypse

This comment has been minimized.

Copy link
Contributor Author

@Ekopalypse Ekopalypse commented Oct 13, 2019

You mean one PR for messages and one for the notifications?

@pryrt

This comment has been minimized.

Copy link
Contributor

@pryrt pryrt commented Oct 13, 2019

Just one total is fine.

If you were editing multiple docs for unrelated reasons, that would be multiple PR. But if it's all edits to the same doc, it's easier for everyone for one PR

@Ekopalypse

This comment has been minimized.

Copy link
Contributor Author

@Ekopalypse Ekopalypse commented Oct 13, 2019

Another Q:
NPPM_ messages do have a space between each message.
For me this looks much more readable than the notifications which are "glued" together.
Should we add an additional spacing to each notification as well?

UPDATE:
I'm talking about the space between the __COMMENT__of NPPM_SETSMOOTFONT and NPPM_SETSTATUSBAR compared to the block of NPPN_READY glued to NPPN_TBMODIFICATION

image

@pryrt

This comment has been minimized.

Copy link
Contributor

@pryrt pryrt commented Oct 13, 2019

Go ahead. (I don't think it needs the space to be readable, but don't have a strong opinion against the space. And consistency is probably a good idea.)

@pryrt

This comment has been minimized.

Copy link
Contributor

@pryrt pryrt commented Oct 16, 2019

While you're at it, @Ekopalypse , you might want to look at forum#18375, where the NPPM_INTERNAL_* messages, such as #define NPPM_INTERNAL_SETFILENAME (NPPMSG + 63), were mentioned as being undocumented as well

@pryrt

This comment has been minimized.

Copy link
Contributor

@pryrt pryrt commented Oct 16, 2019

I tend to agree with @pnedev in the forum, in that they were probably marked as "internal" for a reason. It might be nice to know the reason, but because they aren't part of the public API for plugin messages, they don't need to be documented. (I didn't realize at the time that the internal messages were from a different header file.)

@Ekopalypse

This comment has been minimized.

Copy link
Contributor Author

@Ekopalypse Ekopalypse commented Oct 16, 2019

Just wanted to comment on the forum post but I agree too, only official API should be documented.
From my limited understanding about API design - you keep your internal implementation hidden to
be as flexible as possible. If there is a need to redesign something, you just have to take care that this is transparent to the official API.

@Ekopalypse

This comment has been minimized.

Copy link
Contributor Author

@Ekopalypse Ekopalypse commented Oct 17, 2019

@pryrt

would you mind having a look at https://github.com/Ekopalypse/npp-usermanual/blob/more_messages_and_notifications/content/docs/plugin-communication.md

I added a few things on the introduction, then I added a table
which explains the table layout used by the following messages.
Note, the new layout starts with the message NPPM_ADDTOOLBARICON.
The current layout is used for comparison.

There are two issues which I'm unsure how to handle.
1st. the table doesn't use the whole width always.
I tried setting different width values but without getting the expected result.
Do you have any idea?

2nd. What should be done regarding the documentation and the two open npp issues? See TODO ...

In general, the layout is still a bit messy - see, for example, NPPM_DMMREGASDCKDLG and NPPM_GETNBOPENFILES
compared to most of the others.
Btw. should we use normal text style to inform about the messages and notifications?
Meaning ending a sentence with a dot and starting with a uppercase letter etc...?

Anything else?

Thank you

@pryrt

This comment has been minimized.

Copy link
Contributor

@pryrt pryrt commented Oct 17, 2019

  1. Table width = I noticed that too while doing my updates to that page, but I don't think it's real. Did you download the Hugo app (extended version) from the link in the repo's README? When I rendered my edits with that, rather than with GitHub's rendering engine, the table widths came into line. (GitHub and Hugo use different flavors of markdown engines, so it's probably a processing difference between the two; or a stylesheet difference.)

    • I'm wondering whether the presence or lack of <TBODY> is influencing it: the old-style uses the TBODY whereas your new ones don't, and your new ones render with the gap to the side of the table in GitHub (haven't downloaded your version to try in Hugo, sorry).
    • I do like your conversion from wParam: | [in] desc to in | desc, because it avoids repeating the headers for every entry.
  2. Regarding open issues: documentation should match what's currently there. When the change gets implemented, documentation should be updated to match.

  3. Messy: some of them just have more complicated info that needs to be communicated. I like the compact table-layout for the NPPM_GETNBOPENFILES (except that the header rows got stripped to text on individual lines; that should be fixed)... but I'm not sure whether the structures in NPPM_SAVESESSION and NPPM_DMMREGASDCKDLG would really work better as a table or left as-is.

  4. Periods and capitals: Statements/sentences should definitely start with uppercase. I'm less certain about periods at the end, as I often forget them in the more bullet-style of prose prevalent throughout the documentation. If you were to add periods to them all, I wouldn't complain. (If you removed all the periods, I probably would complain.) Probably best to use periods.

@Ekopalypse

This comment has been minimized.

Copy link
Contributor Author

@Ekopalypse Ekopalypse commented Oct 21, 2019

Yes, I'm using Hugo extended locally to see how it renders, but it seems it is random behavior.
For example see this screenshot

image

and now compare with this one

image

I don't get it.

Concerning tbody, my understanding is that it is not needed but if used it should be used with thead and tfoot.

I'm open for any suggestion, even one which starts with "get rid of inline html ..."

@pryrt

This comment has been minimized.

Copy link
Contributor

@pryrt pryrt commented Oct 21, 2019

Weird. When I render with hugo in Chrome for

<table border="1" width="100%">
<tbody>
<tr>
<td width="50%" rowspan="2" style="background:LightYellow;">  NPPM_GETBUFFERLANGTYPE
</td>
<td width="10%" style="background:khaki"> wParam:
</td>
<td width="30%" style="background:SeaShell;"> 0
</td></tr>
<tr>
<td width="10%" style="background:khaki"> lParam:
</td>
<td>  0
</td></tr>
<tr>
<td colspan="3" style="background:#E8F8E8;">  Get document's language type from given buffer ID.
<p>Returns value&nbsp;: if error -1, otherwise language type (see LangType).
[in] int bufferID
</p>
</td>
</tr>
</tbody></table>

<table border="1" width="100%">
<tbody>
<tr>
<td width="50%" rowspan="2" style="background:LightYellow;">  NPPM_SETBUFFERLANGTYPE
</td>
<td width="10%" style="background:khaki"> wParam:
</td>
<td>  [in] int bufferID
</td></tr>
<tr>
<td width="10%" style="background:khaki"> lParam:
</td>
<td>  [in] LangType langType2Set
</td></tr>
<tr>
<td colspan="3" style="background:#E8F8E8;">  Set language type of given buffer ID's document.
<p>Returns TRUE on success, FALSE otherwise.
L_USER and L_EXTERNAL are not supported.
</p>
</td>
</tr>
</tbody></table>

I get
image

Even when I add the 30% and SeaShell to the lparam,

<table border="1" width="100%">
<tbody>
<tr>
<td width="50%" rowspan="2" style="background:LightYellow;">  NPPM_GETBUFFERLANGTYPE
</td>
<td width="10%" style="background:khaki"> wParam:
</td>
<td width="30%" style="background:SeaShell;"> 0
</td></tr>
<tr>
<td width="10%" style="background:khaki"> lParam:
</td>
<td>  0
</td></tr>
<tr>
<td colspan="3" style="background:#E8F8E8;">  Get document's language type from given buffer ID.
<p>Returns value&nbsp;: if error -1, otherwise language type (see LangType).
[in] int bufferID
</p>
</td>
</tr>
</tbody></table>

<table border="1" width="100%">
<tbody>
<tr>
<td width="50%" rowspan="2" style="background:LightYellow;">  NPPM_SETBUFFERLANGTYPE
</td>
<td width="10%" style="background:khaki"> wParam:
</td>
<td width="30%" style="background:SeaShell;">  [in] int bufferID
</td></tr>
<tr>
<td width="10%" style="background:khaki"> lParam:
</td>
<td>  [in] LangType langType2Set
</td></tr>
<tr>
<td colspan="3" style="background:#E8F8E8;">  Set language type of given buffer ID's document.
<p>Returns TRUE on success, FALSE otherwise.
L_USER and L_EXTERNAL are not supported.
</p>
</td>
</tr>
</tbody></table>

I get
image

So I don't know why yours is rendering differently. Different browser, maybe?

-----

... tbody ...

I know it's not required, but I thought maybe it was affecting the rendering engine in some subtle way.

"get rid of inline html ..."

I'd be more tempted to "switch from .md to .html", but I don't really like that idea (I think hugo can handle mixed media in the different files, but haven't tried it).

These subtle issues are why I went with "good enough to get the content across", rather than "wow, that's all consistent and looks great".

Maybe starting over with an inline-html table structure that's more consistent, and uses classes for styling the td rather than manual width and style attributes. But I don't know if that would solve it.

Sorry I'm not more help on this part.

@Ekopalypse

This comment has been minimized.

Copy link
Contributor Author

@Ekopalypse Ekopalypse commented Oct 22, 2019

Thank you very much for your time and thoughts about this.

So I don't know why yours is rendering differently. Different browser, maybe?

Yes, you are right and I did some more tests and it seems the current solution is not only rendered differently by using different browsers but also by different display resolutions.
And by that I don't mean some nasty little 100x100, but, on my main display I have 1920x1080 whereas my second monitor, which I'm using to view the results, is having 1280x1024 and it looks way better, not perfect, on the main display as on the second display.
Webpage was loaded full screen.

Maybe starting over with an inline-html table structure that's more consistent, and uses classes for
styling the td rather than manual width and style attributes. But I don't know if that would solve it.

Haha, that's what I thought yesterday as well and I'm using currently something like

<style>
/* table layout*/
table {
  table-layout: fixed;
  width: 100%;
  border: 1px solid black;
}

tr {
  width: 100%;
}

/* Message columns are based on these configurations */
td.nppm {
  width: 50%;
  background:LightYellow;
}
td.inout {
  width: 10%;
  background: khaki;
  text-align: center;
}
td.wparam {
  width: 40%;
  background:SeaShell;
}
td.lparam {
  width: 40%;
}
td.nppm_desc {
  width: 100%;
  background: #E8F8E8;
}

/* Notification columns are based on these configurations */
td.nppn {
  width: 90%;
  background:LightYellow;
}
td.hwndFrom {
  width: 20%;
}
td.idFrom {
  width: 20%;
  background:SeaShell;
}
td.nppn_desc {
  width: 100%;
  background: #E8F8E8;
}
</style>

<table>
<tr>
<td rowspan="2" class="nppm">Message</td>
<td class="inout">in</td>
<td class="wparam">wParam</td>
</tr>
<tr>
<td class="inout">inout</td>
<td class="lparam">lParam</td>
</tr>
<tr>
<td colspan="3"  class="nppm_desc">Description</td>
</tr>
</table>

didn't solve the issue but makes further changes easier to implement :-)

I guess I'm creating a pure html solution to see when it starts to break, to get a better
understanding what is going on.

@pryrt pryrt added the in-progress label Nov 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.