Page MenuHomeFreeBSD

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'
Needs ReviewPublic

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

Details

Reviewers
None
Group Reviewers
manpages
bhyve
Summary
PR:             246121
Submitted by:   koinec@yahoo.co.jp
MFC after:      2 weeks

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

mr requested review of this revision.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
163

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?

164

The use of "you" is discouraged. You could write "When using a VNC client..., there is no need to ..."

165

Same here.

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'.
mr changed the repository for this revision from rS FreeBSD src repository - subversion to R10 FreeBSD src repository.

Update bhyve.8 to reflect comments.

OK for the man page part of the change.

grehan added inline comments.
usr.sbin/bhyve/bhyve.8
166

"the layout defaults"
Avoids repetition.

usr.sbin/bhyve/kbdlayout/am
4

Should be "Created by:" in all these files.

usr.sbin/bhyve/bhyve.8
164

The bracketed text could be deleted.

usr.sbin/bhyve/kbdlayout/Makefile
3

There needs to be a corresponding entry for this directory in etc/mtree/BSD.usr.dist

usr.sbin/bhyve/kbdlayout/by
1

The empty layout files could be deleted.

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
8

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