Append Keyboard Layout specified option for using VNC. Part two: Append bhyve -K option for specified keyboard layout with layout setting files every languages. Since the cmd option '-k' was used in the meantime I changed it to '-K'
ClosedPublic
Actions

Authored by mr on Mar 28 2021, 6:57 PM.

Details

Reviewers

None

Group Reviewers

manpages
bhyve

Commits

rGcc820887ab0e: Append Keyboard Layout specified option for using VNC.
rG19eaa01beaba: Append Keyboard Layout specified option for using VNC.

Summary

PR:             246121
Submitted by:   koinec@yahoo.co.jp
MFC after:      2 weeks

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

mr created this revision.Mar 28 2021, 6:57 PM

Herald added a reviewer: manpages. · View Herald TranscriptMar 28 2021, 6:58 PM

Herald added a reviewer: bhyve. · View Herald Transcript

Herald added subscribers: Contributor Reviews (src), bcran, rgrimes, imp. · View Herald Transcript

mr requested review of this revision.Mar 28 2021, 6:58 PM

Harbormaster completed remote builds in B38143: Diff 86446.Mar 28 2021, 6:58 PM

I only checked the doc portion of the change. I generally like this feature and look forward to using it myself.
A few suggestions on the text. Did you check the manpage with "mandoc -Tlint" and textproc/igor? They usually give some good recommendations.

usr.sbin/bhyve/bhyve.8
180	You need a line break after the sentence stop. Maybe you can also put the sentence in () in the first one or describe a bit more why (or in which circumstances) it is not working to make it a sentence of its own?
181	The use of "you" is discouraged. You could write "When using a VNC client..., there is no need to ..."
182	Same here.

mr mentioned this in D29430: Append Keyboard Layout specified option for using VNC. Part one: supporting QEMU Extended Keyboard Event Message.Apr 10 2021, 12:14 PM

mr retitled this revision from Append Keyboard Layout specified option for using VNC. Part two: Append bhyve -K option for specified keyboard layout with layout setting files every languages. Since the cmd option '-k' was used in the meantime I changed it to '-K' to Append Keyboard Layout specified option for using VNC. Part two: Append bhyve -K option for specified keyboard layout with layout setting files every languages. Since the cmd option '-k' was used in the meantime I changed it to '-K'.

Update bhyve.8 to reflect comments.

OK for the man page part of the change.

grehan added a subscriber: grehan.May 24 2021, 7:59 AM

grehan added inline comments.

usr.sbin/bhyve/bhyve.8
166	"the layout defaults" Avoids repetition.
usr.sbin/bhyve/kbdlayout/am
5	Should be "Created by:" in all these files.

grehan added inline comments.May 24 2021, 11:15 AM

usr.sbin/bhyve/bhyve.8
164	The bracketed text could be deleted.
usr.sbin/bhyve/kbdlayout/Makefile
4	There needs to be a corresponding entry for this directory in `etc/mtree/BSD.usr.dist`
usr.sbin/bhyve/kbdlayout/by
1 ↗	(On Diff #87199)	The empty layout files could be deleted.

jhb added a subscriber: jhb.May 24 2021, 5:03 PM

jhb added inline comments.

usr.sbin/bhyve/bhyverun.c
1272	This should be setting a config variable instead so that config files can set it, and the code to fetch it should not use the global variable, but fetch the relevant config setting instead. You'd also need to document the new config variable in bhyve_config.5.
usr.sbin/bhyve/kbdlayout/jp_capsctrl
9	I would perhaps use 'Keys' instead of 'Key' when adding a heading for multiple entries.
usr.sbin/bhyve/ps2kbd.c
435	Use MAX_PATHNAME (or whatever the right constant is)
443	This is probably simpler as a single call to snprintf(). strlcat would let you avoid the complicated length expression, but using snprintf() is even simpler.

An updated diff sent by Koine Yuusuke (koinec@yahoo.co.jp) to address the comments

This revision was not accepted when it landed; it landed in state Needs Review.Jan 20 2022, 10:50 PM

Closed by commit rG19eaa01beaba: Append Keyboard Layout specified option for using VNC. (authored by mr). · Explain Why

This revision was automatically updated to reflect the committed changes.

mr added a commit: rG19eaa01beaba: Append Keyboard Layout specified option for using VNC..

pauamma_gundo.com added a subscriber: pauamma_gundo.com.Jan 21 2022, 7:05 AM

pauamma_gundo.com added inline comments.

usr.sbin/bhyve/bhyve.8
178–179	Do you mean that only values that match filenames in that directory are valid?
179	Per https://docs.freebsd.org/en/books/fdp-primer/manual-pages/#manual-pages-markup (last item in section 10.3.5.2. See also mdoc(7).
182	For consistency with capitalization below.
usr.sbin/bhyve/bhyve_config.5
137–138	See my comments about -K in bhyve(8) above.
140