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'
ClosedPublic

Authored by mr on Mar 28 2021, 6:57 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Apr 9, 7:00 AM
Unknown Object (File)
Tue, Apr 2, 12:07 PM
Unknown Object (File)
Sat, Mar 30, 1:29 AM
Unknown Object (File)
Mon, Mar 25, 2:36 PM
Unknown Object (File)
Mon, Mar 25, 5:52 AM
Unknown Object (File)
Mar 19 2024, 4:44 AM
Unknown Object (File)
Mar 19 2024, 2:34 AM
Unknown Object (File)
Jan 14 2024, 9:44 AM

Details

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 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
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 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 rG 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
5

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
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 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
This revision was automatically updated to reflect the committed changes.
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