bug#26417: 25.2; Add current-line in simple.el

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#26417: 25.2; Add current-line in simple.el

Damien Cassou-2
Hi,

attached patch adds `current-line' and its tests. This new
function returns the line number at given position ignoring
narrowing.

I guess that `what-line' could be refactored using
`current-line'. I can do that if you want.

Best,

--
Damien Cassou
http://damiencassou.seasidehosting.st

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

0001-Add-current-line-in-simple.el.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#26417: 25.2; Add current-line in simple.el

Nicolas Petton-2
Damien Cassou <[hidden email]> writes:

> Hi,

Hi Damien,

> attached patch adds `current-line' and its tests. This new
> function returns the line number at given position ignoring
> narrowing.

I know narrowing have no effect on `current-column', but does it make
sense to widen the buffer for `current-line'? (I don't have a strong
opinion, but I think it's worth asking the question).

Other than that, I think this is useful, thanks for the patch.

> I guess that `what-line' could be refactored using
> `current-line'. I can do that if you want.

Yes, that'd be good.

Cheers,
Nico

signature.asc (482 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#26417: 25.2; Add current-line in simple.el

Damien Cassou-2
Nicolas Petton <[hidden email]> writes:
> I know narrowing have no effect on `current-column', but does it
> make sense to widen the buffer for `current-line'? (I don't have
> a strong opinion, but I think it's worth asking the question).

The existing function `line-number-at-pos` returns the line number
relative to current narrowing. I need a function that returns the
absolute line number (as I need to save the line number in a
separate file so that I can later open at the appropriate
line). Do you think the function name is misleading? What about
one of these?

- line-number-at-pos-absolute
- current-absolute-line


--
Damien Cassou
http://damiencassou.seasidehosting.st

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#26417: 25.2; Add current-line in simple.el

Nicolas Petton-2
Damien Cassou <[hidden email]> writes:

> Nicolas Petton <[hidden email]> writes:
>> I know narrowing have no effect on `current-column', but does it
>> make sense to widen the buffer for `current-line'? (I don't have
>> a strong opinion, but I think it's worth asking the question).
>
> The existing function `line-number-at-pos` returns the line number
> relative to current narrowing. I need a function that returns the
> absolute line number (as I need to save the line number in a
> separate file so that I can later open at the appropriate
> line). Do you think the function name is misleading? What about
> one of these?
What about something like this:

    (defun current-line (&optional ignore-narrowing)
      ...)

Cheers,
Nico

signature.asc (482 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#26417: 25.2; Add current-line in simple.el

Damien Cassou-2
Nicolas Petton <[hidden email]> writes:
> What about something like this:
>
>     (defun current-line (&optional ignore-narrowing)
>       ...)

thanks for your suggestion. I applied it in attached patch. I'm
not sure I like it though because the simplest call:

    (current-line)

is now exactly the same as (line-number-at-pos). To get a
different behavior, non-nil must be passed as argument:

    (current-line t)

Nevertheless, I understand this implementation is probably the one
closest to existing API and usages.

Best,

--
Damien Cassou
http://damiencassou.seasidehosting.st

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

0001-Add-current-line-in-simple.el.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#26417: 25.2; Add current-line in simple.el

Nicolas Petton-2
Damien Cassou <[hidden email]> writes:

> thanks for your suggestion. I applied it in attached patch. I'm
> not sure I like it though because the simplest call:
>
>     (current-line)
>
> is now exactly the same as (line-number-at-pos). To get a
> different behavior, non-nil must be passed as argument:

Well, it would be the same as (line-number-at-pos (point)), yes, but I
think it's the most consistent implementation WRT `current-column'.

Cheers,
Nico

signature.asc (482 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#26417: 25.2; Add current-line in simple.el

Damien Cassou-2
Nicolas Petton <[hidden email]> writes:
> Well, it would be the same as (line-number-at-pos (point))

which is the same as (line-number-at-pos) according to
documentation.


>, yes, but I think it's the most consistent implementation WRT
>`current-column'.

then merge it :-).

--
Damien Cassou
http://damiencassou.seasidehosting.st

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#26417: 25.2; Add current-line in simple.el

Nicolas Petton-2
Damien Cassou <[hidden email]> writes:

> which is the same as (line-number-at-pos) according to
> documentation.

Indeed.  What about adding the optional argument to `line-number-at-pos'
then?

Nico

signature.asc (482 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#26417: 25.2; Add current-line in simple.el

Damien Cassou-2
Nicolas Petton <[hidden email]> writes:
> Indeed.  What about adding the optional argument to
> `line-number-at-pos' then?

ok. Here is another version following your piece of advice.

--
Damien Cassou
http://damiencassou.seasidehosting.st

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

0001-Add-current-line-in-simple.el.patch (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#26417: 25.2; Add current-line in simple.el

Damien Cassou-2
Damien Cassou <[hidden email]> writes:
> ok. Here is another version following your piece of advice.  

up.

--
Damien Cassou
http://damiencassou.seasidehosting.st

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#26417: 25.2; Add current-line in simple.el

Nicolas Petton-2
Damien Cassou <[hidden email]> writes:

> up.

Looks good to me, thanks!

I'll install it in master in a few days unless someone has comments
regarding this patch.

Cheers,
Nico

signature.asc (482 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

bug#26417: 25.2; Add current-line in simple.el

Nicolas Petton-2
In reply to this post by Damien Cassou-2
Damien Cassou <[hidden email]> writes:

> ok. Here is another version following your piece of advice.

I installed your patch in master, thanks!

Cheers,
Nico

signature.asc (482 bytes) Download Attachment
Loading...