matlab-emacs-discuss

[Matlab-emacs-discuss] [patch] Issue with indentation of 'end'

[Thibault M. <thi...@ie...>]

Hi all,

I have an issue with the indentation of 'end' lines with matlab.el.  It
seems that the calculated indentation level does not get multiplied by the
indent size resulting in blocks like:

if true
	if true
		for i = 1:10
			disp(1);
  end
 end
end

Am I the only one to experience this?  The attached patch fixes it for
me but I am not sure whether this is a bug or an issue with my setup.

Any input would be welcome, thanks

Re: [Matlab-emacs-discuss] [patch] Issue with indentation of 'end'

[Uwe B. <ou...@ma...>]

>>> "Thibault" == Thibault Marin <thi...@ie...> writes:

    > Hi all,

Hi

BTW I applied your first patch and pushed it. So this second one is
already for the version with your former patch applied?
Till the end of the week I have no time to look into this hopefully at
the weekended.

I have a faint memory of a problem with if, but right now I can't
remember.

regards

Uwe 

Re: [Matlab-emacs-discuss] [patch] Issue with indentation of 'end'

[Thibault M. <thi...@ie...>]

Uwe Brauer writes:

>>>> "Thibault" == Thibault Marin <thi...@ie...> writes:
>
>     > Hi all,
>
> Hi
>
> BTW I applied your first patch and pushed it. So this second one is
> already for the version with your former patch applied?
> Till the end of the week I have no time to look into this hopefully at
> the weekended.
>
> I have a faint memory of a problem with if, but right now I can't
> remember.
>
> regards
>
> Uwe 

I did a git pull after my first patch was merged, so it should be
up-to-date.  Thanks for looking into it.

Re: [Matlab-emacs-discuss] [patch] Issue with indentation of 'end'

[Eric L. <Eri...@ma...>]

I believe that code is using absolute positioning (ie - matched positions in the buffer) so the calculated indent ci should have the actual amount to indent baked in.

I note you have TABs in your pasted output.  Since matlab.el is doing character calculations, that will mess it up.   Since matlab-mode sets indent-tabs-mode to nil, this shouldn't happen unless you've overridden that feature.

Eric

-----Original Message-----
From: Thibault Marin [mailto:thi...@ie...] 
Sent: Wednesday, June 01, 2016 10:10 AM
To: mat...@li...
Subject: [Matlab-emacs-discuss] [patch] Issue with indentation of 'end'


Hi all,

I have an issue with the indentation of 'end' lines with matlab.el.  It seems that the calculated indentation level does not get multiplied by the indent size resulting in blocks like:

if true
	if true
		for i = 1:10
			disp(1);
  end
 end
end

Am I the only one to experience this?  The attached patch fixes it for me but I am not sure whether this is a bug or an issue with my setup.

Any input would be welcome, thanks

Re: [Matlab-emacs-discuss] [patch] Issue with indentation of 'end'

[Thibault M. <thi...@gm...>]

Eric Ludlam writes:

> I believe that code is using absolute positioning (ie - matched positions in the buffer) so the calculated indent ci should have the actual amount to indent baked in.
>
> I note you have TABs in your pasted output.  Since matlab.el is doing character calculations, that will mess it up.   Since matlab-mode sets indent-tabs-mode to nil, this shouldn't happen unless you've overridden that feature.
>
> Eric
>
> -----Original Message-----
> From: Thibault Marin [mailto:thi...@ie...] 
> Sent: Wednesday, June 01, 2016 10:10 AM
> To: mat...@li...
> Subject: [Matlab-emacs-discuss] [patch] Issue with indentation of 'end'
>
>
> Hi all,
>
> I have an issue with the indentation of 'end' lines with matlab.el.  It seems that the calculated indentation level does not get multiplied by the indent size resulting in blocks like:
>
> if true
> 	if true
> 		for i = 1:10
> 			disp(1);
>   end
>  end
> end
>
> Am I the only one to experience this?  The attached patch fixes it for me but I am not sure whether this is a bug or an issue with my setup.
>
> Any input would be welcome, thanks

Thanks, the problem is that I am using tabs.  Everything works fine when
indent-tabs-mode is nil.

However, I would like to use tabs if possible and indentation of the
'end' seems to be the only hurdle in supporting this.  So, would a patch
on the vein of the attached one be acceptable?

Re: [Matlab-emacs-discuss] [patch] Issue with indentation of 'end'

[Eric L. <Eri...@ma...>]

Hi,

I would guess that 'end' indentation is not really the last hurdle.  For TABs to work, all places where character math occur will need to be replaced with calls to 'current-column'.

Ie - instead of

(- match (match-beginning 0))

You would need

(- match-col (save-excursion (goto-char (match-beginning 0)) (current-column))

Current-column is used in a bunch of places already, so hopefully the number of problem areas is small.

Eric

-----Original Message-----
From: Thibault Marin [mailto:thi...@gm...] 
Sent: Thursday, June 02, 2016 12:05 PM
To: Eric Ludlam <Eri...@ma...>
Cc: thi...@gm...; mat...@li...
Subject: Re: [Matlab-emacs-discuss] [patch] Issue with indentation of 'end'


Eric Ludlam writes:

> I believe that code is using absolute positioning (ie - matched positions in the buffer) so the calculated indent ci should have the actual amount to indent baked in.
>
> I note you have TABs in your pasted output.  Since matlab.el is doing character calculations, that will mess it up.   Since matlab-mode sets indent-tabs-mode to nil, this shouldn't happen unless you've overridden that feature.
>
> Eric
>
> -----Original Message-----
> From: Thibault Marin [mailto:thi...@ie...]
> Sent: Wednesday, June 01, 2016 10:10 AM
> To: mat...@li...
> Subject: [Matlab-emacs-discuss] [patch] Issue with indentation of 'end'
>
>
> Hi all,
>
> I have an issue with the indentation of 'end' lines with matlab.el.  It seems that the calculated indentation level does not get multiplied by the indent size resulting in blocks like:
>
> if true
> 	if true
> 		for i = 1:10
> 			disp(1);
>   end
>  end
> end
>
> Am I the only one to experience this?  The attached patch fixes it for me but I am not sure whether this is a bug or an issue with my setup.
>
> Any input would be welcome, thanks

Thanks, the problem is that I am using tabs.  Everything works fine when indent-tabs-mode is nil.

However, I would like to use tabs if possible and indentation of the 'end' seems to be the only hurdle in supporting this.  So, would a patch on the vein of the attached one be acceptable?

Re: [Matlab-emacs-discuss] [patch] Issue with indentation of 'end'

[Thibault M. <thi...@gm...>]

Eric Ludlam writes:

> Hi,
>
> I would guess that 'end' indentation is not really the last hurdle.  For TABs to work, all places where character math occur will need to be replaced with calls to 'current-column'.
>
> Ie - instead of
>
> (- match (match-beginning 0))
>
> You would need
>
> (- match-col (save-excursion (goto-char (match-beginning 0)) (current-column))
>
> Current-column is used in a bunch of places already, so hopefully the number of problem areas is small.
>
> Eric
>

Thanks for the guidance.

I modified the patch to use (current-column) as advised.  Could you
please let me know if this is what you had in mind?

I understand that there may be other places where a similar change
should be done, but tabs seem to be working fine for me at this point: I
haven't found any indentation problems after fixing the 'end' line.  If
other issues arise, I can try to have a look.

Re: [Matlab-emacs-discuss] [patch] Issue with indentation of 'end'

[Eric L. <Eri...@ma...>]

Your patch looks good to me by inspection.   I'm glad it is working out for you.

Eric

-----Original Message-----
From: Thibault Marin [mailto:thi...@gm...] 
Sent: Thursday, June 02, 2016 10:56 PM
To: Eric Ludlam <Eri...@ma...>
Cc: thi...@gm...valid; mat...@li...
Subject: Re: [Matlab-emacs-discuss] [patch] Issue with indentation of 'end'


Eric Ludlam writes:

> Hi,
>
> I would guess that 'end' indentation is not really the last hurdle.  For TABs to work, all places where character math occur will need to be replaced with calls to 'current-column'.
>
> Ie - instead of
>
> (- match (match-beginning 0))
>
> You would need
>
> (- match-col (save-excursion (goto-char (match-beginning 0)) 
> (current-column))
>
> Current-column is used in a bunch of places already, so hopefully the number of problem areas is small.
>
> Eric
>

Thanks for the guidance.

I modified the patch to use (current-column) as advised.  Could you please let me know if this is what you had in mind?

I understand that there may be other places where a similar change should be done, but tabs seem to be working fine for me at this point: I haven't found any indentation problems after fixing the 'end' line.  If other issues arise, I can try to have a look.

Re: [Matlab-emacs-discuss] [patch] Issue with indentation of 'end'

[Uwe B. <ou...@ma...>]

>>> "Eric" == Eric Ludlam <Eri...@ma...> writes:

    > Your patch looks good to me by inspection.   I'm glad it is working out for you.
    > Eric

Hi

Thanks Eric for looking into that. Now that this questions seems to be
settled shall I apply and commit the last patch?

Uwe 

Re: [Matlab-emacs-discuss] [patch] Issue with indentation of 'end'

[Thibault M. <thi...@gm...>]

As far as I am concerned, the patch fixes my issue, and I think it
doesn't break the old behavior, so I think it would be good if it could
be merged.

Please let me know if the patch needs modifications to make that happen.

Thanks.

Uwe Brauer writes:

>>>> "Eric" == Eric Ludlam <Eri...@ma...> writes:
>
>     > Your patch looks good to me by inspection.   I'm glad it is working out for you.
>     > Eric
>
> Hi
>
> Thanks Eric for looking into that. Now that this questions seems to be
> settled shall I apply and commit the last patch?
>
> Uwe