Re: gcs doubt #1

Re: gcs doubt #1

[adrian15]

>>> If you can provide a patch for sourcecode to improve or add comments
>>> in a sane way, I am willing to commit those patches.  Just please
>>> don't make the mistake to add useless comments.
>> An useful comment is one that:
>> 	-Describes a function algorithm
>> 	-Describes what a function does (as a black box)
>> 	-Boot of them ?
> 
> Yes, but I oppose to adding obvious comments.  But patches will be
> reviewed, so you will learn fast enough what we want :-).

You had to choose one of the three options :).
Well... it doesn't matter. I've understood that I have to send patches
and learn by trial-error.

adrian15

Re: test -e patch

[adrian15]

>>> adrian15 <adrian15@raulete.net> writes:
>>>
>>>>> Attached you will find the patch adding test -e support for grub2.
>> Do you mean you also have the '-e' option ?
> 
> I mean I have everything that is possible.  That includes `-e', but
> also all other features.
Ok.
> 
>>>>> +static void
>>>>> +test_file_exists (const char *key)
>>> Why not filename?
>> test_filename_exists
>> or
>> filename
> 
> I mean instead of key.

You are right. It was a not clean copy-paste from search command.

adrian15

Re: grub2 miscelanea questions (1/2)

[adrian15]

>> 3) Why any grub2.cfg file (I suppose it's the substitute for menu.lst)
>> is inside the grub2 source code?
> 
> What do you mean?

Usually a program should include examples on how to use their files and
document examples and so on... why isn't there any grub2.cfg ?

>> 4) Why any doc folder is inside the grub2 source code?
> 
> There is?  We need documentation...
:) I might work on this too... but as an unofficial documentation. I do
not like the rigid official documentations.
>  	
>> 5) grub2 lacks
>> ==================
>> 5.1) pager
>> -------------
>>
>> When I want to do a big ls the output does not stop screen by screen...
>> Is there any hidden pager that I do not know?
>> Maybe is not implemented yet?
> 
> set pager=1
> 
> It just isn't documented :-/

We should be able to prompt with help command (help variables perhaps?)
these kind of tricks or pseudocommands.

> 
>> 5.2) Chainload DEVICES
>> ------------------------
>>
>> How the hell do you boot windows?
>> Maybe from a file that has inside it the first 512 bytes from the
>> Windows partition?
> 
> chainload (hd0,1)+
> boot

I've checked that this works:
chainloader (hd0,1)+1
boot

>> 6) Compulsory Argument dilemma
>> ================================	
>> If I run help I can seen things like this:
>> 	
>> ls [-f|-l|-s] NAME
>>
>> and
>>
>> [ EXPRESSION ]
> 
> Yes?
> 
>> If [ ] means optional it should also mean optional in the case of the
>> test alias [ .!!!
> 
> What do you mean?

The [] has not a single meaning in this program.
	If it's written after a command in the help description it means: Optional.
	If it's used as a program it means that it runs the test command.

It might be confusing for some users I suppose.

Why did not use to new bash syntax? $((variable))

> 
>> 7) set help incomplete
>> ========================
>> Set help should say that if no argument is set then it shows all the
>> environment variables.
> 
> Why would it?  Only using set shows all env. variables.
Because it is a hidden feature.
If you do not how grub does work and type "help set"
it shows "Set an environment variable".

The user is unable to know that "Only using set shows all env.
variables" because it isn't written anywhere.

>> 9) help does not complain when no command help is available
>> =============================================================
> How it currently works is looking at the first characters.  For
> example try "help l".

I know... but if the first characters do not match with any command
beginning I think an error should be prompted. Don't you think so?

> 
>> 10) help --all
>> =================
>> Do we need this functionality in grub2?
> 
> Are there hidden commands for which this is important?  In that case,
> yes.

The hipotetical dd command. :)
> 
>> 12) ls and -h argument
>> ========================
>>
>> This is not an error but it curious. You can ask for the help of command
>> with this:
>> command -h
>> however
>> ls -h
>> works in a different way.
>>
>> I suppose we may have the same problem with '-u' in the future.
> 
> Ehm, I am not sure.  You mean `-h' is short for help and we should
> remove it?

I mean that if an user gets used to use the "-h" option for getting grub
commands help then when using ls, he will be confused a little.

adrian15

Re: grub2 miscelanea questions (2/2)

[adrian15]

>> 19) rescue and exit failure on qemu
>> ==============================
>> I think that booting from my computer does not give this error.
>> If I boot from QEMU and type:
>>
>> grub> rescue
>> and then :
>> grub rescue> exit
>>
>> it says: FATAL: INT18: BOOT FAILURE
> 
> Weird.  Can you debug this?

I've checked this again.
If I do this booting from my pc I get the grub2 floppy booting again.
If I do this from qemu the FATAL: INT18: BOOT FAILURE appears again.
I would bet that it is a qemu bios's lack of the 18 interruption.
>> 20) grub rescue help
>> ========================
>>
>> help : Types the commands and a minimal explanation: OK
>> help any_command : Types the commands and a minimal explanation: NOT OK
>> help not_a_command : Types the commands and a minimal explanation: NOT OK
> 
> huh?

You boot grub2 and you see
grub>
you type rescue and you see
grub rescue>
and now what it is funny that if you type the help command, an space,
and whatever you want to write it always shows all the commands help.

It may confuse some users as "Grub is not working".

> 
>> 21) grub rescue exit help
>> ===========================
>> When you are in grub rescue mode and you type help the exit line it's
>> the following one:
>>
>> exit	exit from GRUB
>>
>> Should it not prompt ?
>>
>> exit	exit from GRUB rescue
> 
> The exit command terminates GRUB and continues the boot process, IIRC.
You are right. Exit from GRUB is right, then.

> 
>> 22) copyright message missing
>> ================================
>> Why isn't there (when Grub is running) any message about the GRUB
>> Copyright and the GPL license and that you should have received and this
>> and that?
> 
> Why should there be such message?
This comes from GPL 2:

If the program is interactive, make it output a short notice like this
when it starts in an interactive mode:

     Gnomovision version 69, Copyright (C) year  name of author
     Gnomovision comes with ABSOLUTELY NO WARRANTY; for details type
`show w'.
     This is free software, and you are welcome to redistribute it
     under certain conditions; type `show c' for details.

>> Maybe we should implement an 'about' command and run it when booting :) ?
> Perhaps, but I don't see the added value.
See above about the GPL.
> 
>> 23) set read only variables
>> ==============================
>> unset prefix lets me "delete" the prefix variable.
>> unset root does not let me the "root" variable.
>>
>> I remember that bash has an option to set read only variables.
>> Maybe the root variable is a read only variable?
> 
> You can easily modify the code.  But I do not like making such
> variables read-only.

I did not want to make any variable read only. I only wanted to make you
know that the "root" variable was actually a read only variable because
there is no way to delete (unset) it. It always comes to live again.

Is it a normal thing?

adrian15

gcs doubt #1

[adrian15]

In the GNU Coding Standards they say in:

5.2 Commenting Your Work

Please put a comment on each function saying what the function does,
what sorts of arguments it gets, and what the possible values of
arguments mean and are used for.


Is it ok that the search.c and test.c (commands/ folder) do not have any
of these comments at all?

You do not comment them at all?
You comment them in another place?
Maybe it's ok with the help description that it is set when you register
a command?
Maybe this is a bug and should be fixed?

adrian15

Re: gcs doubt #1

[Marco Gerards]

adrian15 <adrian15@raulete.net> writes:

> Please put a comment on each function saying what the function does,
> what sorts of arguments it gets, and what the possible values of
> arguments mean and are used for.
>
> Is it ok that the search.c and test.c (commands/ folder) do not have any
> of these comments at all?

The currect test.c is a dummy so the `if' expression can be
used/tested.

If you can provide a patch for sourcecode to improve or add comments
in a sane way, I am willing to commit those patches.  Just please
don't make the mistake to add useless comments.

--
Marco

Re: gcs doubt #1

[adrian15]

>> > Please put a comment on each function saying what the function does,
>> > what sorts of arguments it gets, and what the possible values of
>> > arguments mean and are used for.
>> >
>> > Is it ok that the search.c and test.c (commands/ folder) do not have any
>> > of these comments at all?
> 
> The currect test.c is a dummy so the `if' expression can be
> used/tested.
> 
> If you can provide a patch for sourcecode to improve or add comments
> in a sane way, I am willing to commit those patches.  Just please
> don't make the mistake to add useless comments.

An useful comment is one that:
	-Describes a function algorithm
	-Describes what a function does (as a black box)
	-Boot of them ?

adrian15

Re: gcs doubt #1

[Marco Gerards]

adrian15 <adrian15@raulete.net> writes:

>>> > Please put a comment on each function saying what the function does,
>>> > what sorts of arguments it gets, and what the possible values of
>>> > arguments mean and are used for.
>>> >
>>> > Is it ok that the search.c and test.c (commands/ folder) do not have any
>>> > of these comments at all?
>>
>> The currect test.c is a dummy so the `if' expression can be
>> used/tested.
>>
>> If you can provide a patch for sourcecode to improve or add comments
>> in a sane way, I am willing to commit those patches.  Just please
>> don't make the mistake to add useless comments.
>
> An useful comment is one that:
> 	-Describes a function algorithm
> 	-Describes what a function does (as a black box)
> 	-Boot of them ?

Yes, but I oppose to adding obvious comments.  But patches will be
reviewed, so you will learn fast enough what we want :-).

--
Marco